* [PATCH] clk: Add device tree binding to clk-fixed-factor @ 2013-04-10 15:40 Christian Ruppert 2013-04-10 15:56 ` Gregory CLEMENT 0 siblings, 1 reply; 12+ messages in thread From: Christian Ruppert @ 2013-04-10 15:40 UTC (permalink / raw) To: linux-kernel, linux-arm-kernel Cc: Mike Turquette, linux-doc, devicetree-discuss, Rob Herring, Grant Likely, Rob Landley, Christian Ruppert This patch adds a device tree binding for the simple fixed factor clock divider/multiplier of the common clock tree binding. Signed-off-by: Christian Ruppert <christian.ruppert@abilis.com> --- .../bindings/clock/fixed-factor-clkdiv.txt | 24 +++++++++++++++ drivers/clk/clk-fixed-factor.c | 32 ++++++++++++++++++++ include/linux/clk-provider.h | 1 + 3 files changed, 57 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/clock/fixed-factor-clkdiv.txt diff --git a/Documentation/devicetree/bindings/clock/fixed-factor-clkdiv.txt b/Documentation/devicetree/bindings/clock/fixed-factor-clkdiv.txt new file mode 100644 index 0000000..352bac4 --- /dev/null +++ b/Documentation/devicetree/bindings/clock/fixed-factor-clkdiv.txt @@ -0,0 +1,24 @@ +Device Tree Clock bindings for plat-tb10x + +This binding uses the common clock binding[1]. + +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt + +Required properties: +- compatible : shall be "fixed-factor-clkdiv" +- #clock-cells: from common clock binding; shall be set to 0 +- clocks: shall be the input parent clock phandle for the clock. +- clock-mult: defines the multiplication factor of the output clock frequency + wrt. the input clock frequency. +- clock-div: defines the division factor of the output clock frequency wrt. + the input clock frequency. + +Example: +cpu_clk: clkdiv_cpu { /* CPU clock derived from pll0. 1/2 of pll frequency */ + compatible = "fixed-factor-clkdiv"; + #clock-cells = <0>; + clocks = <&pll0>; + clock-mult = <1>; + clock-div = <2>; + clock-output-names = "cpu_clk"; +}; diff --git a/drivers/clk/clk-fixed-factor.c b/drivers/clk/clk-fixed-factor.c index 1ef271e..85e45f1 100644 --- a/drivers/clk/clk-fixed-factor.c +++ b/drivers/clk/clk-fixed-factor.c @@ -11,6 +11,7 @@ #include <linux/clk-provider.h> #include <linux/slab.h> #include <linux/err.h> +#include <linux/of.h> /* * DOC: basic fixed multiplier and divider clock that cannot gate @@ -96,3 +97,34 @@ struct clk *clk_register_fixed_factor(struct device *dev, const char *name, return clk; } + +#ifdef CONFIG_OF +/** + * of_fixed_factor_clkdiv_setup() - Set up simple fixed factor clock divider + */ +void of_fixed_factor_clkdiv_setup(struct device_node *node) +{ + struct clk *clk; + const char *clk_name = node->name; + const char *parent_name; + u32 mult, div; + + if (of_property_read_u32(node, "clock-mult", &mult)) + return; + + if (of_property_read_u32(node, "clock-div", &div)) + return; + + parent_name = of_clk_get_parent_name(node, 0); + + of_property_read_string(node, "clock-output-names", &clk_name); + + clk = clk_register_fixed_factor(NULL, clk_name, parent_name, 0, + mult, div); + if (!IS_ERR(clk)) + of_clk_add_provider(node, of_clk_src_simple_get, clk); +} +EXPORT_SYMBOL_GPL(of_fixed_factor_clkdiv_setup); +CLK_OF_DECLARE(fixed_clkdiv, "fixed-factor-clkdiv", + of_fixed_factor_clkdiv_setup); +#endif diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h index 7f197d7..d4937cf 100644 --- a/include/linux/clk-provider.h +++ b/include/linux/clk-provider.h @@ -184,6 +184,7 @@ struct clk *clk_register_fixed_rate(struct device *dev, const char *name, unsigned long fixed_rate); void of_fixed_clk_setup(struct device_node *np); +void of_fixed_factor_clkdiv_setup(struct device_node *node); /** * struct clk_gate - gating clock -- 1.7.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] clk: Add device tree binding to clk-fixed-factor 2013-04-10 15:40 [PATCH] clk: Add device tree binding to clk-fixed-factor Christian Ruppert @ 2013-04-10 15:56 ` Gregory CLEMENT 2013-04-10 16:27 ` Christian Ruppert 0 siblings, 1 reply; 12+ messages in thread From: Gregory CLEMENT @ 2013-04-10 15:56 UTC (permalink / raw) To: Christian Ruppert Cc: linux-kernel, linux-arm-kernel, Mike Turquette, linux-doc, devicetree-discuss, Rob Herring, Grant Likely, Rob Landley Hi Christian, On 04/10/2013 05:40 PM, Christian Ruppert wrote: > This patch adds a device tree binding for the simple fixed factor clock > divider/multiplier of the common clock tree binding. This patch remind me of something : http://lists.infradead.org/pipermail/linux-arm-kernel/2012-August/116405.html Do you have a use case for it, this time? My version have been hold off waiting for an user for it. Will you need it for a driver? Regards, > > Signed-off-by: Christian Ruppert <christian.ruppert@abilis.com> > --- > .../bindings/clock/fixed-factor-clkdiv.txt | 24 +++++++++++++++ > drivers/clk/clk-fixed-factor.c | 32 ++++++++++++++++++++ > include/linux/clk-provider.h | 1 + > 3 files changed, 57 insertions(+), 0 deletions(-) > create mode 100644 Documentation/devicetree/bindings/clock/fixed-factor-clkdiv.txt > > diff --git a/Documentation/devicetree/bindings/clock/fixed-factor-clkdiv.txt b/Documentation/devicetree/bindings/clock/fixed-factor-clkdiv.txt > new file mode 100644 > index 0000000..352bac4 > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/fixed-factor-clkdiv.txt > @@ -0,0 +1,24 @@ > +Device Tree Clock bindings for plat-tb10x > + > +This binding uses the common clock binding[1]. > + > +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt > + > +Required properties: > +- compatible : shall be "fixed-factor-clkdiv" > +- #clock-cells: from common clock binding; shall be set to 0 > +- clocks: shall be the input parent clock phandle for the clock. > +- clock-mult: defines the multiplication factor of the output clock frequency > + wrt. the input clock frequency. > +- clock-div: defines the division factor of the output clock frequency wrt. > + the input clock frequency. > + > +Example: > +cpu_clk: clkdiv_cpu { /* CPU clock derived from pll0. 1/2 of pll frequency */ > + compatible = "fixed-factor-clkdiv"; > + #clock-cells = <0>; > + clocks = <&pll0>; > + clock-mult = <1>; > + clock-div = <2>; > + clock-output-names = "cpu_clk"; > +}; > diff --git a/drivers/clk/clk-fixed-factor.c b/drivers/clk/clk-fixed-factor.c > index 1ef271e..85e45f1 100644 > --- a/drivers/clk/clk-fixed-factor.c > +++ b/drivers/clk/clk-fixed-factor.c > @@ -11,6 +11,7 @@ > #include <linux/clk-provider.h> > #include <linux/slab.h> > #include <linux/err.h> > +#include <linux/of.h> > > /* > * DOC: basic fixed multiplier and divider clock that cannot gate > @@ -96,3 +97,34 @@ struct clk *clk_register_fixed_factor(struct device *dev, const char *name, > > return clk; > } > + > +#ifdef CONFIG_OF > +/** > + * of_fixed_factor_clkdiv_setup() - Set up simple fixed factor clock divider > + */ > +void of_fixed_factor_clkdiv_setup(struct device_node *node) > +{ > + struct clk *clk; > + const char *clk_name = node->name; > + const char *parent_name; > + u32 mult, div; > + > + if (of_property_read_u32(node, "clock-mult", &mult)) > + return; > + > + if (of_property_read_u32(node, "clock-div", &div)) > + return; > + > + parent_name = of_clk_get_parent_name(node, 0); > + > + of_property_read_string(node, "clock-output-names", &clk_name); > + > + clk = clk_register_fixed_factor(NULL, clk_name, parent_name, 0, > + mult, div); > + if (!IS_ERR(clk)) > + of_clk_add_provider(node, of_clk_src_simple_get, clk); > +} > +EXPORT_SYMBOL_GPL(of_fixed_factor_clkdiv_setup); > +CLK_OF_DECLARE(fixed_clkdiv, "fixed-factor-clkdiv", > + of_fixed_factor_clkdiv_setup); > +#endif > diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h > index 7f197d7..d4937cf 100644 > --- a/include/linux/clk-provider.h > +++ b/include/linux/clk-provider.h > @@ -184,6 +184,7 @@ struct clk *clk_register_fixed_rate(struct device *dev, const char *name, > unsigned long fixed_rate); > > void of_fixed_clk_setup(struct device_node *np); > +void of_fixed_factor_clkdiv_setup(struct device_node *node); > > /** > * struct clk_gate - gating clock > -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] clk: Add device tree binding to clk-fixed-factor 2013-04-10 15:56 ` Gregory CLEMENT @ 2013-04-10 16:27 ` Christian Ruppert 2013-04-11 9:19 ` Christian Ruppert 0 siblings, 1 reply; 12+ messages in thread From: Christian Ruppert @ 2013-04-10 16:27 UTC (permalink / raw) To: Gregory CLEMENT Cc: linux-kernel, linux-arm-kernel, Mike Turquette, linux-doc, devicetree-discuss, Rob Herring, Grant Likely, Rob Landley Hi Gregory, We use derived clocks with fixed ratios for our TB10x platform. I originally had this in the platform code but Vineet (the platform maintainer) rightly pointed out that the common clock framework would be the correct place for it. Is there any reason not to support this functionality? After all, the generic fixed rate clock also has a device tree binding and I'd guess we're not the only ones statically deriving one clock from another in our SOCs. Greetings, Christian On Wed, Apr 10, 2013 at 05:56:25PM +0200, Gregory CLEMENT wrote: > Hi Christian, > > On 04/10/2013 05:40 PM, Christian Ruppert wrote: > > This patch adds a device tree binding for the simple fixed factor clock > > divider/multiplier of the common clock tree binding. > > This patch remind me of something : > http://lists.infradead.org/pipermail/linux-arm-kernel/2012-August/116405.html > > Do you have a use case for it, this time? > > My version have been hold off waiting for an user for it. > Will you need it for a driver? > > Regards, > > > > > Signed-off-by: Christian Ruppert <christian.ruppert@abilis.com> > > --- > > .../bindings/clock/fixed-factor-clkdiv.txt | 24 +++++++++++++++ > > drivers/clk/clk-fixed-factor.c | 32 ++++++++++++++++++++ > > include/linux/clk-provider.h | 1 + > > 3 files changed, 57 insertions(+), 0 deletions(-) > > create mode 100644 Documentation/devicetree/bindings/clock/fixed-factor-clkdiv.txt > > > > diff --git a/Documentation/devicetree/bindings/clock/fixed-factor-clkdiv.txt b/Documentation/devicetree/bindings/clock/fixed-factor-clkdiv.txt > > new file mode 100644 > > index 0000000..352bac4 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/clock/fixed-factor-clkdiv.txt > > @@ -0,0 +1,24 @@ > > +Device Tree Clock bindings for plat-tb10x > > + > > +This binding uses the common clock binding[1]. > > + > > +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt > > + > > +Required properties: > > +- compatible : shall be "fixed-factor-clkdiv" > > +- #clock-cells: from common clock binding; shall be set to 0 > > +- clocks: shall be the input parent clock phandle for the clock. > > +- clock-mult: defines the multiplication factor of the output clock frequency > > + wrt. the input clock frequency. > > +- clock-div: defines the division factor of the output clock frequency wrt. > > + the input clock frequency. > > + > > +Example: > > +cpu_clk: clkdiv_cpu { /* CPU clock derived from pll0. 1/2 of pll frequency */ > > + compatible = "fixed-factor-clkdiv"; > > + #clock-cells = <0>; > > + clocks = <&pll0>; > > + clock-mult = <1>; > > + clock-div = <2>; > > + clock-output-names = "cpu_clk"; > > +}; > > diff --git a/drivers/clk/clk-fixed-factor.c b/drivers/clk/clk-fixed-factor.c > > index 1ef271e..85e45f1 100644 > > --- a/drivers/clk/clk-fixed-factor.c > > +++ b/drivers/clk/clk-fixed-factor.c > > @@ -11,6 +11,7 @@ > > #include <linux/clk-provider.h> > > #include <linux/slab.h> > > #include <linux/err.h> > > +#include <linux/of.h> > > > > /* > > * DOC: basic fixed multiplier and divider clock that cannot gate > > @@ -96,3 +97,34 @@ struct clk *clk_register_fixed_factor(struct device *dev, const char *name, > > > > return clk; > > } > > + > > +#ifdef CONFIG_OF > > +/** > > + * of_fixed_factor_clkdiv_setup() - Set up simple fixed factor clock divider > > + */ > > +void of_fixed_factor_clkdiv_setup(struct device_node *node) > > +{ > > + struct clk *clk; > > + const char *clk_name = node->name; > > + const char *parent_name; > > + u32 mult, div; > > + > > + if (of_property_read_u32(node, "clock-mult", &mult)) > > + return; > > + > > + if (of_property_read_u32(node, "clock-div", &div)) > > + return; > > + > > + parent_name = of_clk_get_parent_name(node, 0); > > + > > + of_property_read_string(node, "clock-output-names", &clk_name); > > + > > + clk = clk_register_fixed_factor(NULL, clk_name, parent_name, 0, > > + mult, div); > > + if (!IS_ERR(clk)) > > + of_clk_add_provider(node, of_clk_src_simple_get, clk); > > +} > > +EXPORT_SYMBOL_GPL(of_fixed_factor_clkdiv_setup); > > +CLK_OF_DECLARE(fixed_clkdiv, "fixed-factor-clkdiv", > > + of_fixed_factor_clkdiv_setup); > > +#endif > > diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h > > index 7f197d7..d4937cf 100644 > > --- a/include/linux/clk-provider.h > > +++ b/include/linux/clk-provider.h > > @@ -184,6 +184,7 @@ struct clk *clk_register_fixed_rate(struct device *dev, const char *name, > > unsigned long fixed_rate); > > > > void of_fixed_clk_setup(struct device_node *np); > > +void of_fixed_factor_clkdiv_setup(struct device_node *node); > > > > /** > > * struct clk_gate - gating clock > > > > > -- > Gregory Clement, Free Electrons > Kernel, drivers, real-time and embedded Linux > development, consulting, training and support. > http://free-electrons.com -- Christian Ruppert , <christian.ruppert@abilis.com> /| Tel: +41/(0)22 816 19-42 //| 3, Chemin du Pré-Fleuri _// | bilis Systems CH-1228 Plan-les-Ouates ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] clk: Add device tree binding to clk-fixed-factor 2013-04-10 16:27 ` Christian Ruppert @ 2013-04-11 9:19 ` Christian Ruppert 2013-04-11 16:26 ` Gregory CLEMENT 0 siblings, 1 reply; 12+ messages in thread From: Christian Ruppert @ 2013-04-11 9:19 UTC (permalink / raw) To: Gregory CLEMENT Cc: linux-kernel, linux-arm-kernel, Mike Turquette, linux-doc, devicetree-discuss, Rob Herring, Grant Likely, Rob Landley, Vineet Gupta Hi Gregory, Since there doesn't seem to be anyone opposing this feature I just checked your patch and after adding the CLK_OF_DECLARE macro in the end it works well for us. Tell me if you would like to rebase and resubmit your patch or if I should follow up on our own. Greetings, Christian On Wed, Apr 10, 2013 at 06:27:47PM +0200, christian ruppert wrote: > Hi Gregory, > > We use derived clocks with fixed ratios for our TB10x platform. I > originally had this in the platform code but Vineet (the platform > maintainer) rightly pointed out that the common clock framework would > be the correct place for it. > Is there any reason not to support this functionality? After all, the > generic fixed rate clock also has a device tree binding and I'd guess > we're not the only ones statically deriving one clock from another in > our SOCs. > > Greetings, > Christian > > On Wed, Apr 10, 2013 at 05:56:25PM +0200, Gregory CLEMENT wrote: > > Hi Christian, > > > > On 04/10/2013 05:40 PM, Christian Ruppert wrote: > > > This patch adds a device tree binding for the simple fixed factor clock > > > divider/multiplier of the common clock tree binding. > > > > This patch remind me of something : > > http://lists.infradead.org/pipermail/linux-arm-kernel/2012-August/116405.html > > > > Do you have a use case for it, this time? > > > > My version have been hold off waiting for an user for it. > > Will you need it for a driver? > > > > Regards, > > > > > > > > Signed-off-by: Christian Ruppert <christian.ruppert@abilis.com> > > > --- > > > .../bindings/clock/fixed-factor-clkdiv.txt | 24 +++++++++++++++ > > > drivers/clk/clk-fixed-factor.c | 32 ++++++++++++++++++++ > > > include/linux/clk-provider.h | 1 + > > > 3 files changed, 57 insertions(+), 0 deletions(-) > > > create mode 100644 Documentation/devicetree/bindings/clock/fixed-factor-clkdiv.txt > > > > > > diff --git a/Documentation/devicetree/bindings/clock/fixed-factor-clkdiv.txt b/Documentation/devicetree/bindings/clock/fixed-factor-clkdiv.txt > > > new file mode 100644 > > > index 0000000..352bac4 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/clock/fixed-factor-clkdiv.txt > > > @@ -0,0 +1,24 @@ > > > +Device Tree Clock bindings for plat-tb10x > > > + > > > +This binding uses the common clock binding[1]. > > > + > > > +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt > > > + > > > +Required properties: > > > +- compatible : shall be "fixed-factor-clkdiv" > > > +- #clock-cells: from common clock binding; shall be set to 0 > > > +- clocks: shall be the input parent clock phandle for the clock. > > > +- clock-mult: defines the multiplication factor of the output clock frequency > > > + wrt. the input clock frequency. > > > +- clock-div: defines the division factor of the output clock frequency wrt. > > > + the input clock frequency. > > > + > > > +Example: > > > +cpu_clk: clkdiv_cpu { /* CPU clock derived from pll0. 1/2 of pll frequency */ > > > + compatible = "fixed-factor-clkdiv"; > > > + #clock-cells = <0>; > > > + clocks = <&pll0>; > > > + clock-mult = <1>; > > > + clock-div = <2>; > > > + clock-output-names = "cpu_clk"; > > > +}; > > > diff --git a/drivers/clk/clk-fixed-factor.c b/drivers/clk/clk-fixed-factor.c > > > index 1ef271e..85e45f1 100644 > > > --- a/drivers/clk/clk-fixed-factor.c > > > +++ b/drivers/clk/clk-fixed-factor.c > > > @@ -11,6 +11,7 @@ > > > #include <linux/clk-provider.h> > > > #include <linux/slab.h> > > > #include <linux/err.h> > > > +#include <linux/of.h> > > > > > > /* > > > * DOC: basic fixed multiplier and divider clock that cannot gate > > > @@ -96,3 +97,34 @@ struct clk *clk_register_fixed_factor(struct device *dev, const char *name, > > > > > > return clk; > > > } > > > + > > > +#ifdef CONFIG_OF > > > +/** > > > + * of_fixed_factor_clkdiv_setup() - Set up simple fixed factor clock divider > > > + */ > > > +void of_fixed_factor_clkdiv_setup(struct device_node *node) > > > +{ > > > + struct clk *clk; > > > + const char *clk_name = node->name; > > > + const char *parent_name; > > > + u32 mult, div; > > > + > > > + if (of_property_read_u32(node, "clock-mult", &mult)) > > > + return; > > > + > > > + if (of_property_read_u32(node, "clock-div", &div)) > > > + return; > > > + > > > + parent_name = of_clk_get_parent_name(node, 0); > > > + > > > + of_property_read_string(node, "clock-output-names", &clk_name); > > > + > > > + clk = clk_register_fixed_factor(NULL, clk_name, parent_name, 0, > > > + mult, div); > > > + if (!IS_ERR(clk)) > > > + of_clk_add_provider(node, of_clk_src_simple_get, clk); > > > +} > > > +EXPORT_SYMBOL_GPL(of_fixed_factor_clkdiv_setup); > > > +CLK_OF_DECLARE(fixed_clkdiv, "fixed-factor-clkdiv", > > > + of_fixed_factor_clkdiv_setup); > > > +#endif > > > diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h > > > index 7f197d7..d4937cf 100644 > > > --- a/include/linux/clk-provider.h > > > +++ b/include/linux/clk-provider.h > > > @@ -184,6 +184,7 @@ struct clk *clk_register_fixed_rate(struct device *dev, const char *name, > > > unsigned long fixed_rate); > > > > > > void of_fixed_clk_setup(struct device_node *np); > > > +void of_fixed_factor_clkdiv_setup(struct device_node *node); > > > > > > /** > > > * struct clk_gate - gating clock > > > > > > > > > -- > > Gregory Clement, Free Electrons > > Kernel, drivers, real-time and embedded Linux > > development, consulting, training and support. > > http://free-electrons.com > > -- > Christian Ruppert , <christian.ruppert@abilis.com> > /| > Tel: +41/(0)22 816 19-42 //| 3, Chemin du Pré-Fleuri > _// | bilis Systems CH-1228 Plan-les-Ouates -- Christian Ruppert , <christian.ruppert@abilis.com> /| Tel: +41/(0)22 816 19-42 //| 3, Chemin du Pré-Fleuri _// | bilis Systems CH-1228 Plan-les-Ouates ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] clk: Add device tree binding to clk-fixed-factor 2013-04-11 9:19 ` Christian Ruppert @ 2013-04-11 16:26 ` Gregory CLEMENT 2013-04-12 6:54 ` Christian Ruppert 0 siblings, 1 reply; 12+ messages in thread From: Gregory CLEMENT @ 2013-04-11 16:26 UTC (permalink / raw) To: Christian Ruppert Cc: linux-kernel, linux-arm-kernel, Mike Turquette, linux-doc, devicetree-discuss, Rob Herring, Grant Likely, Rob Landley, Vineet Gupta On 04/11/2013 11:19 AM, Christian Ruppert wrote: > Hi Gregory, > > Since there doesn't seem to be anyone opposing this feature I just > checked your patch and after adding the CLK_OF_DECLARE macro in the end > it works well for us. Tell me if you would like to rebase and resubmit > your patch or if I should follow up on our own. I can rebase and resubmit my patch, it's pretty straightforward. The problem was the lack of user in the kernel. And by user I mean a driver using these functions. Usually a new function enter in the kernel only if there are users for it inside the kernel so if you plan to submit a driver using this piece of code, then I see no reason to not get it. > > Greetings, > Christian > > On Wed, Apr 10, 2013 at 06:27:47PM +0200, christian ruppert wrote: >> Hi Gregory, >> >> We use derived clocks with fixed ratios for our TB10x platform. I >> originally had this in the platform code but Vineet (the platform >> maintainer) rightly pointed out that the common clock framework would >> be the correct place for it. >> Is there any reason not to support this functionality? After all, the >> generic fixed rate clock also has a device tree binding and I'd guess >> we're not the only ones statically deriving one clock from another in >> our SOCs. >> >> Greetings, >> Christian >> >> On Wed, Apr 10, 2013 at 05:56:25PM +0200, Gregory CLEMENT wrote: >>> Hi Christian, >>> >>> On 04/10/2013 05:40 PM, Christian Ruppert wrote: >>>> This patch adds a device tree binding for the simple fixed factor clock >>>> divider/multiplier of the common clock tree binding. >>> >>> This patch remind me of something : >>> http://lists.infradead.org/pipermail/linux-arm-kernel/2012-August/116405.html >>> >>> Do you have a use case for it, this time? >>> >>> My version have been hold off waiting for an user for it. >>> Will you need it for a driver? >>> >>> Regards, >>> >>>> >>>> Signed-off-by: Christian Ruppert <christian.ruppert@abilis.com> >>>> --- >>>> .../bindings/clock/fixed-factor-clkdiv.txt | 24 +++++++++++++++ >>>> drivers/clk/clk-fixed-factor.c | 32 ++++++++++++++++++++ >>>> include/linux/clk-provider.h | 1 + >>>> 3 files changed, 57 insertions(+), 0 deletions(-) >>>> create mode 100644 Documentation/devicetree/bindings/clock/fixed-factor-clkdiv.txt >>>> >>>> diff --git a/Documentation/devicetree/bindings/clock/fixed-factor-clkdiv.txt b/Documentation/devicetree/bindings/clock/fixed-factor-clkdiv.txt >>>> new file mode 100644 >>>> index 0000000..352bac4 >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/clock/fixed-factor-clkdiv.txt >>>> @@ -0,0 +1,24 @@ >>>> +Device Tree Clock bindings for plat-tb10x >>>> + >>>> +This binding uses the common clock binding[1]. >>>> + >>>> +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt >>>> + >>>> +Required properties: >>>> +- compatible : shall be "fixed-factor-clkdiv" >>>> +- #clock-cells: from common clock binding; shall be set to 0 >>>> +- clocks: shall be the input parent clock phandle for the clock. >>>> +- clock-mult: defines the multiplication factor of the output clock frequency >>>> + wrt. the input clock frequency. >>>> +- clock-div: defines the division factor of the output clock frequency wrt. >>>> + the input clock frequency. >>>> + >>>> +Example: >>>> +cpu_clk: clkdiv_cpu { /* CPU clock derived from pll0. 1/2 of pll frequency */ >>>> + compatible = "fixed-factor-clkdiv"; >>>> + #clock-cells = <0>; >>>> + clocks = <&pll0>; >>>> + clock-mult = <1>; >>>> + clock-div = <2>; >>>> + clock-output-names = "cpu_clk"; >>>> +}; >>>> diff --git a/drivers/clk/clk-fixed-factor.c b/drivers/clk/clk-fixed-factor.c >>>> index 1ef271e..85e45f1 100644 >>>> --- a/drivers/clk/clk-fixed-factor.c >>>> +++ b/drivers/clk/clk-fixed-factor.c >>>> @@ -11,6 +11,7 @@ >>>> #include <linux/clk-provider.h> >>>> #include <linux/slab.h> >>>> #include <linux/err.h> >>>> +#include <linux/of.h> >>>> >>>> /* >>>> * DOC: basic fixed multiplier and divider clock that cannot gate >>>> @@ -96,3 +97,34 @@ struct clk *clk_register_fixed_factor(struct device *dev, const char *name, >>>> >>>> return clk; >>>> } >>>> + >>>> +#ifdef CONFIG_OF >>>> +/** >>>> + * of_fixed_factor_clkdiv_setup() - Set up simple fixed factor clock divider >>>> + */ >>>> +void of_fixed_factor_clkdiv_setup(struct device_node *node) >>>> +{ >>>> + struct clk *clk; >>>> + const char *clk_name = node->name; >>>> + const char *parent_name; >>>> + u32 mult, div; >>>> + >>>> + if (of_property_read_u32(node, "clock-mult", &mult)) >>>> + return; >>>> + >>>> + if (of_property_read_u32(node, "clock-div", &div)) >>>> + return; >>>> + >>>> + parent_name = of_clk_get_parent_name(node, 0); >>>> + >>>> + of_property_read_string(node, "clock-output-names", &clk_name); >>>> + >>>> + clk = clk_register_fixed_factor(NULL, clk_name, parent_name, 0, >>>> + mult, div); >>>> + if (!IS_ERR(clk)) >>>> + of_clk_add_provider(node, of_clk_src_simple_get, clk); >>>> +} >>>> +EXPORT_SYMBOL_GPL(of_fixed_factor_clkdiv_setup); >>>> +CLK_OF_DECLARE(fixed_clkdiv, "fixed-factor-clkdiv", >>>> + of_fixed_factor_clkdiv_setup); >>>> +#endif >>>> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h >>>> index 7f197d7..d4937cf 100644 >>>> --- a/include/linux/clk-provider.h >>>> +++ b/include/linux/clk-provider.h >>>> @@ -184,6 +184,7 @@ struct clk *clk_register_fixed_rate(struct device *dev, const char *name, >>>> unsigned long fixed_rate); >>>> >>>> void of_fixed_clk_setup(struct device_node *np); >>>> +void of_fixed_factor_clkdiv_setup(struct device_node *node); >>>> >>>> /** >>>> * struct clk_gate - gating clock >>>> >>> >>> >>> -- >>> Gregory Clement, Free Electrons >>> Kernel, drivers, real-time and embedded Linux >>> development, consulting, training and support. >>> http://free-electrons.com >> >> -- >> Christian Ruppert , <christian.ruppert@abilis.com> >> /| >> Tel: +41/(0)22 816 19-42 //| 3, Chemin du Pré-Fleuri >> _// | bilis Systems CH-1228 Plan-les-Ouates > -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] clk: Add device tree binding to clk-fixed-factor 2013-04-11 16:26 ` Gregory CLEMENT @ 2013-04-12 6:54 ` Christian Ruppert 2013-04-12 7:05 ` Gregory CLEMENT 2013-04-12 9:04 ` Gregory CLEMENT 0 siblings, 2 replies; 12+ messages in thread From: Christian Ruppert @ 2013-04-12 6:54 UTC (permalink / raw) To: Gregory CLEMENT Cc: linux-kernel, linux-arm-kernel, Mike Turquette, linux-doc, devicetree-discuss, Rob Herring, Grant Likely, Rob Landley, Vineet Gupta On Thu, Apr 11, 2013 at 06:26:07PM +0200, Gregory CLEMENT wrote: > On 04/11/2013 11:19 AM, Christian Ruppert wrote: > > Hi Gregory, > > > > Since there doesn't seem to be anyone opposing this feature I just > > checked your patch and after adding the CLK_OF_DECLARE macro in the end > > it works well for us. Tell me if you would like to rebase and resubmit > > your patch or if I should follow up on our own. > > I can rebase and resubmit my patch, it's pretty straightforward. > The problem was the lack of user in the kernel. And by user I mean > a driver using these functions. Usually a new function enter in the > kernel only if there are users for it inside the kernel so if you plan > to submit a driver using this piece of code, then I see no reason > to not get it. If you add the line CLK_OF_DECLARE(fixed_factor_clk, "fixed-factor-clkdiv", in the end of drivers/clk/clk-fixed-factor.c, the new function will be automatically used by of_clk_init(NULL) which itself is used in numerous platform ports, in particular arch/arc/plat-tb10x. plat-tb10x is currently being integrated in the ARC architecture tree. The device tree file requiring this patch is already in linux-next (see https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/arch/arc/boot/dts/abilis_*.dtsi). We didn't know if you wanted to resubmit your patch or if I should pursue mine at the time, and today it uses the naming scheme from mine. It would be nice if you could either adapt your patch to that naming scheme. Greetings, Christian > > Greetings, > > Christian > > > > On Wed, Apr 10, 2013 at 06:27:47PM +0200, christian ruppert wrote: > >> Hi Gregory, > >> > >> We use derived clocks with fixed ratios for our TB10x platform. I > >> originally had this in the platform code but Vineet (the platform > >> maintainer) rightly pointed out that the common clock framework would > >> be the correct place for it. > >> Is there any reason not to support this functionality? After all, the > >> generic fixed rate clock also has a device tree binding and I'd guess > >> we're not the only ones statically deriving one clock from another in > >> our SOCs. > >> > >> Greetings, > >> Christian > >> > >> On Wed, Apr 10, 2013 at 05:56:25PM +0200, Gregory CLEMENT wrote: > >>> Hi Christian, > >>> > >>> On 04/10/2013 05:40 PM, Christian Ruppert wrote: > >>>> This patch adds a device tree binding for the simple fixed factor clock > >>>> divider/multiplier of the common clock tree binding. > >>> > >>> This patch remind me of something : > >>> http://lists.infradead.org/pipermail/linux-arm-kernel/2012-August/116405.html > >>> > >>> Do you have a use case for it, this time? > >>> > >>> My version have been hold off waiting for an user for it. > >>> Will you need it for a driver? > >>> > >>> Regards, > >>> > >>>> > >>>> Signed-off-by: Christian Ruppert <christian.ruppert@abilis.com> > >>>> --- > >>>> .../bindings/clock/fixed-factor-clkdiv.txt | 24 +++++++++++++++ > >>>> drivers/clk/clk-fixed-factor.c | 32 ++++++++++++++++++++ > >>>> include/linux/clk-provider.h | 1 + > >>>> 3 files changed, 57 insertions(+), 0 deletions(-) > >>>> create mode 100644 Documentation/devicetree/bindings/clock/fixed-factor-clkdiv.txt > >>>> > >>>> diff --git a/Documentation/devicetree/bindings/clock/fixed-factor-clkdiv.txt b/Documentation/devicetree/bindings/clock/fixed-factor-clkdiv.txt > >>>> new file mode 100644 > >>>> index 0000000..352bac4 > >>>> --- /dev/null > >>>> +++ b/Documentation/devicetree/bindings/clock/fixed-factor-clkdiv.txt > >>>> @@ -0,0 +1,24 @@ > >>>> +Device Tree Clock bindings for plat-tb10x > >>>> + > >>>> +This binding uses the common clock binding[1]. > >>>> + > >>>> +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt > >>>> + > >>>> +Required properties: > >>>> +- compatible : shall be "fixed-factor-clkdiv" > >>>> +- #clock-cells: from common clock binding; shall be set to 0 > >>>> +- clocks: shall be the input parent clock phandle for the clock. > >>>> +- clock-mult: defines the multiplication factor of the output clock frequency > >>>> + wrt. the input clock frequency. > >>>> +- clock-div: defines the division factor of the output clock frequency wrt. > >>>> + the input clock frequency. > >>>> + > >>>> +Example: > >>>> +cpu_clk: clkdiv_cpu { /* CPU clock derived from pll0. 1/2 of pll frequency */ > >>>> + compatible = "fixed-factor-clkdiv"; > >>>> + #clock-cells = <0>; > >>>> + clocks = <&pll0>; > >>>> + clock-mult = <1>; > >>>> + clock-div = <2>; > >>>> + clock-output-names = "cpu_clk"; > >>>> +}; > >>>> diff --git a/drivers/clk/clk-fixed-factor.c b/drivers/clk/clk-fixed-factor.c > >>>> index 1ef271e..85e45f1 100644 > >>>> --- a/drivers/clk/clk-fixed-factor.c > >>>> +++ b/drivers/clk/clk-fixed-factor.c > >>>> @@ -11,6 +11,7 @@ > >>>> #include <linux/clk-provider.h> > >>>> #include <linux/slab.h> > >>>> #include <linux/err.h> > >>>> +#include <linux/of.h> > >>>> > >>>> /* > >>>> * DOC: basic fixed multiplier and divider clock that cannot gate > >>>> @@ -96,3 +97,34 @@ struct clk *clk_register_fixed_factor(struct device *dev, const char *name, > >>>> > >>>> return clk; > >>>> } > >>>> + > >>>> +#ifdef CONFIG_OF > >>>> +/** > >>>> + * of_fixed_factor_clkdiv_setup() - Set up simple fixed factor clock divider > >>>> + */ > >>>> +void of_fixed_factor_clkdiv_setup(struct device_node *node) > >>>> +{ > >>>> + struct clk *clk; > >>>> + const char *clk_name = node->name; > >>>> + const char *parent_name; > >>>> + u32 mult, div; > >>>> + > >>>> + if (of_property_read_u32(node, "clock-mult", &mult)) > >>>> + return; > >>>> + > >>>> + if (of_property_read_u32(node, "clock-div", &div)) > >>>> + return; > >>>> + > >>>> + parent_name = of_clk_get_parent_name(node, 0); > >>>> + > >>>> + of_property_read_string(node, "clock-output-names", &clk_name); > >>>> + > >>>> + clk = clk_register_fixed_factor(NULL, clk_name, parent_name, 0, > >>>> + mult, div); > >>>> + if (!IS_ERR(clk)) > >>>> + of_clk_add_provider(node, of_clk_src_simple_get, clk); > >>>> +} > >>>> +EXPORT_SYMBOL_GPL(of_fixed_factor_clkdiv_setup); > >>>> +CLK_OF_DECLARE(fixed_clkdiv, "fixed-factor-clkdiv", > >>>> + of_fixed_factor_clkdiv_setup); > >>>> +#endif > >>>> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h > >>>> index 7f197d7..d4937cf 100644 > >>>> --- a/include/linux/clk-provider.h > >>>> +++ b/include/linux/clk-provider.h > >>>> @@ -184,6 +184,7 @@ struct clk *clk_register_fixed_rate(struct device *dev, const char *name, > >>>> unsigned long fixed_rate); > >>>> > >>>> void of_fixed_clk_setup(struct device_node *np); > >>>> +void of_fixed_factor_clkdiv_setup(struct device_node *node); > >>>> > >>>> /** > >>>> * struct clk_gate - gating clock > >>>> > >>> > >>> > >>> -- > >>> Gregory Clement, Free Electrons > >>> Kernel, drivers, real-time and embedded Linux > >>> development, consulting, training and support. > >>> http://free-electrons.com > >> > >> -- > >> Christian Ruppert , <christian.ruppert@abilis.com> > >> /| > >> Tel: +41/(0)22 816 19-42 //| 3, Chemin du Pré-Fleuri > >> _// | bilis Systems CH-1228 Plan-les-Ouates > > > > > -- > Gregory Clement, Free Electrons > Kernel, drivers, real-time and embedded Linux > development, consulting, training and support. > http://free-electrons.com -- Christian Ruppert , <christian.ruppert@abilis.com> /| Tel: +41/(0)22 816 19-42 //| 3, Chemin du Pré-Fleuri _// | bilis Systems CH-1228 Plan-les-Ouates ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] clk: Add device tree binding to clk-fixed-factor 2013-04-12 6:54 ` Christian Ruppert @ 2013-04-12 7:05 ` Gregory CLEMENT 2013-04-12 9:04 ` Gregory CLEMENT 1 sibling, 0 replies; 12+ messages in thread From: Gregory CLEMENT @ 2013-04-12 7:05 UTC (permalink / raw) To: Christian Ruppert Cc: linux-kernel, linux-arm-kernel, Mike Turquette, linux-doc, devicetree-discuss, Rob Herring, Grant Likely, Rob Landley, Vineet Gupta On 04/12/2013 08:54 AM, Christian Ruppert wrote: > On Thu, Apr 11, 2013 at 06:26:07PM +0200, Gregory CLEMENT wrote: >> On 04/11/2013 11:19 AM, Christian Ruppert wrote: >>> Hi Gregory, >>> >>> Since there doesn't seem to be anyone opposing this feature I just >>> checked your patch and after adding the CLK_OF_DECLARE macro in the end >>> it works well for us. Tell me if you would like to rebase and resubmit >>> your patch or if I should follow up on our own. >> >> I can rebase and resubmit my patch, it's pretty straightforward. >> The problem was the lack of user in the kernel. And by user I mean >> a driver using these functions. Usually a new function enter in the >> kernel only if there are users for it inside the kernel so if you plan >> to submit a driver using this piece of code, then I see no reason >> to not get it. > > If you add the line > > CLK_OF_DECLARE(fixed_factor_clk, "fixed-factor-clkdiv", > > in the end of drivers/clk/clk-fixed-factor.c, the new function will be > automatically used by of_clk_init(NULL) which itself is used in numerous > platform ports, in particular arch/arc/plat-tb10x. plat-tb10x is currently > being integrated in the ARC architecture tree. The device tree file > requiring this patch is already in linux-next (see > https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/arch/arc/boot/dts/abilis_*.dtsi). > great it is good to know there are "user" for this function. > We didn't know if you wanted to resubmit your patch or if I should > pursue mine at the time, and today it uses the naming scheme from mine. > It would be nice if you could either adapt your patch to that naming > scheme. I am going to resubmit my patch this morning and adapt it to your naming scheme. Regards, > > Greetings, > Christian > >>> Greetings, >>> Christian >>> >>> On Wed, Apr 10, 2013 at 06:27:47PM +0200, christian ruppert wrote: >>>> Hi Gregory, >>>> >>>> We use derived clocks with fixed ratios for our TB10x platform. I >>>> originally had this in the platform code but Vineet (the platform >>>> maintainer) rightly pointed out that the common clock framework would >>>> be the correct place for it. >>>> Is there any reason not to support this functionality? After all, the >>>> generic fixed rate clock also has a device tree binding and I'd guess >>>> we're not the only ones statically deriving one clock from another in >>>> our SOCs. >>>> >>>> Greetings, >>>> Christian >>>> >>>> On Wed, Apr 10, 2013 at 05:56:25PM +0200, Gregory CLEMENT wrote: >>>>> Hi Christian, >>>>> >>>>> On 04/10/2013 05:40 PM, Christian Ruppert wrote: >>>>>> This patch adds a device tree binding for the simple fixed factor clock >>>>>> divider/multiplier of the common clock tree binding. >>>>> >>>>> This patch remind me of something : >>>>> http://lists.infradead.org/pipermail/linux-arm-kernel/2012-August/116405.html >>>>> >>>>> Do you have a use case for it, this time? >>>>> >>>>> My version have been hold off waiting for an user for it. >>>>> Will you need it for a driver? >>>>> >>>>> Regards, >>>>> >>>>>> >>>>>> Signed-off-by: Christian Ruppert <christian.ruppert@abilis.com> >>>>>> --- >>>>>> .../bindings/clock/fixed-factor-clkdiv.txt | 24 +++++++++++++++ >>>>>> drivers/clk/clk-fixed-factor.c | 32 ++++++++++++++++++++ >>>>>> include/linux/clk-provider.h | 1 + >>>>>> 3 files changed, 57 insertions(+), 0 deletions(-) >>>>>> create mode 100644 Documentation/devicetree/bindings/clock/fixed-factor-clkdiv.txt >>>>>> >>>>>> diff --git a/Documentation/devicetree/bindings/clock/fixed-factor-clkdiv.txt b/Documentation/devicetree/bindings/clock/fixed-factor-clkdiv.txt >>>>>> new file mode 100644 >>>>>> index 0000000..352bac4 >>>>>> --- /dev/null >>>>>> +++ b/Documentation/devicetree/bindings/clock/fixed-factor-clkdiv.txt >>>>>> @@ -0,0 +1,24 @@ >>>>>> +Device Tree Clock bindings for plat-tb10x >>>>>> + >>>>>> +This binding uses the common clock binding[1]. >>>>>> + >>>>>> +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt >>>>>> + >>>>>> +Required properties: >>>>>> +- compatible : shall be "fixed-factor-clkdiv" >>>>>> +- #clock-cells: from common clock binding; shall be set to 0 >>>>>> +- clocks: shall be the input parent clock phandle for the clock. >>>>>> +- clock-mult: defines the multiplication factor of the output clock frequency >>>>>> + wrt. the input clock frequency. >>>>>> +- clock-div: defines the division factor of the output clock frequency wrt. >>>>>> + the input clock frequency. >>>>>> + >>>>>> +Example: >>>>>> +cpu_clk: clkdiv_cpu { /* CPU clock derived from pll0. 1/2 of pll frequency */ >>>>>> + compatible = "fixed-factor-clkdiv"; >>>>>> + #clock-cells = <0>; >>>>>> + clocks = <&pll0>; >>>>>> + clock-mult = <1>; >>>>>> + clock-div = <2>; >>>>>> + clock-output-names = "cpu_clk"; >>>>>> +}; >>>>>> diff --git a/drivers/clk/clk-fixed-factor.c b/drivers/clk/clk-fixed-factor.c >>>>>> index 1ef271e..85e45f1 100644 >>>>>> --- a/drivers/clk/clk-fixed-factor.c >>>>>> +++ b/drivers/clk/clk-fixed-factor.c >>>>>> @@ -11,6 +11,7 @@ >>>>>> #include <linux/clk-provider.h> >>>>>> #include <linux/slab.h> >>>>>> #include <linux/err.h> >>>>>> +#include <linux/of.h> >>>>>> >>>>>> /* >>>>>> * DOC: basic fixed multiplier and divider clock that cannot gate >>>>>> @@ -96,3 +97,34 @@ struct clk *clk_register_fixed_factor(struct device *dev, const char *name, >>>>>> >>>>>> return clk; >>>>>> } >>>>>> + >>>>>> +#ifdef CONFIG_OF >>>>>> +/** >>>>>> + * of_fixed_factor_clkdiv_setup() - Set up simple fixed factor clock divider >>>>>> + */ >>>>>> +void of_fixed_factor_clkdiv_setup(struct device_node *node) >>>>>> +{ >>>>>> + struct clk *clk; >>>>>> + const char *clk_name = node->name; >>>>>> + const char *parent_name; >>>>>> + u32 mult, div; >>>>>> + >>>>>> + if (of_property_read_u32(node, "clock-mult", &mult)) >>>>>> + return; >>>>>> + >>>>>> + if (of_property_read_u32(node, "clock-div", &div)) >>>>>> + return; >>>>>> + >>>>>> + parent_name = of_clk_get_parent_name(node, 0); >>>>>> + >>>>>> + of_property_read_string(node, "clock-output-names", &clk_name); >>>>>> + >>>>>> + clk = clk_register_fixed_factor(NULL, clk_name, parent_name, 0, >>>>>> + mult, div); >>>>>> + if (!IS_ERR(clk)) >>>>>> + of_clk_add_provider(node, of_clk_src_simple_get, clk); >>>>>> +} >>>>>> +EXPORT_SYMBOL_GPL(of_fixed_factor_clkdiv_setup); >>>>>> +CLK_OF_DECLARE(fixed_clkdiv, "fixed-factor-clkdiv", >>>>>> + of_fixed_factor_clkdiv_setup); >>>>>> +#endif >>>>>> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h >>>>>> index 7f197d7..d4937cf 100644 >>>>>> --- a/include/linux/clk-provider.h >>>>>> +++ b/include/linux/clk-provider.h >>>>>> @@ -184,6 +184,7 @@ struct clk *clk_register_fixed_rate(struct device *dev, const char *name, >>>>>> unsigned long fixed_rate); >>>>>> >>>>>> void of_fixed_clk_setup(struct device_node *np); >>>>>> +void of_fixed_factor_clkdiv_setup(struct device_node *node); >>>>>> >>>>>> /** >>>>>> * struct clk_gate - gating clock >>>>>> >>>>> >>>>> >>>>> -- >>>>> Gregory Clement, Free Electrons >>>>> Kernel, drivers, real-time and embedded Linux >>>>> development, consulting, training and support. >>>>> http://free-electrons.com >>>> >>>> -- >>>> Christian Ruppert , <christian.ruppert@abilis.com> >>>> /| >>>> Tel: +41/(0)22 816 19-42 //| 3, Chemin du Pré-Fleuri >>>> _// | bilis Systems CH-1228 Plan-les-Ouates >>> >> >> >> -- >> Gregory Clement, Free Electrons >> Kernel, drivers, real-time and embedded Linux >> development, consulting, training and support. >> http://free-electrons.com > -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] clk: Add device tree binding to clk-fixed-factor 2013-04-12 6:54 ` Christian Ruppert 2013-04-12 7:05 ` Gregory CLEMENT @ 2013-04-12 9:04 ` Gregory CLEMENT 2013-04-12 9:12 ` Christian Ruppert 1 sibling, 1 reply; 12+ messages in thread From: Gregory CLEMENT @ 2013-04-12 9:04 UTC (permalink / raw) To: Christian Ruppert Cc: linux-kernel, linux-arm-kernel, Mike Turquette, linux-doc, devicetree-discuss, Rob Herring, Grant Likely, Rob Landley, Vineet Gupta On 04/12/2013 08:54 AM, Christian Ruppert wrote: > On Thu, Apr 11, 2013 at 06:26:07PM +0200, Gregory CLEMENT wrote: >> On 04/11/2013 11:19 AM, Christian Ruppert wrote: >>> Hi Gregory, >>> >>> Since there doesn't seem to be anyone opposing this feature I just >>> checked your patch and after adding the CLK_OF_DECLARE macro in the end >>> it works well for us. Tell me if you would like to rebase and resubmit >>> your patch or if I should follow up on our own. >> >> I can rebase and resubmit my patch, it's pretty straightforward. >> The problem was the lack of user in the kernel. And by user I mean >> a driver using these functions. Usually a new function enter in the >> kernel only if there are users for it inside the kernel so if you plan >> to submit a driver using this piece of code, then I see no reason >> to not get it. > > If you add the line > > CLK_OF_DECLARE(fixed_factor_clk, "fixed-factor-clkdiv", Why did you add the div suffix? There is nothing in the function which prevent to have "mul" greater than "div" and hence having a multiplicative factor. I think that this suffix is misleading. > > in the end of drivers/clk/clk-fixed-factor.c, the new function will be > automatically used by of_clk_init(NULL) which itself is used in numerous > platform ports, in particular arch/arc/plat-tb10x. plat-tb10x is currently > being integrated in the ARC architecture tree. The device tree file > requiring this patch is already in linux-next (see > https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/arch/arc/boot/dts/abilis_*.dtsi). > > We didn't know if you wanted to resubmit your patch or if I should > pursue mine at the time, and today it uses the naming scheme from mine. > It would be nice if you could either adapt your patch to that naming > scheme. > > Greetings, > Christian > >>> Greetings, >>> Christian >>> >>> On Wed, Apr 10, 2013 at 06:27:47PM +0200, christian ruppert wrote: >>>> Hi Gregory, >>>> >>>> We use derived clocks with fixed ratios for our TB10x platform. I >>>> originally had this in the platform code but Vineet (the platform >>>> maintainer) rightly pointed out that the common clock framework would >>>> be the correct place for it. >>>> Is there any reason not to support this functionality? After all, the >>>> generic fixed rate clock also has a device tree binding and I'd guess >>>> we're not the only ones statically deriving one clock from another in >>>> our SOCs. >>>> >>>> Greetings, >>>> Christian >>>> >>>> On Wed, Apr 10, 2013 at 05:56:25PM +0200, Gregory CLEMENT wrote: >>>>> Hi Christian, >>>>> >>>>> On 04/10/2013 05:40 PM, Christian Ruppert wrote: >>>>>> This patch adds a device tree binding for the simple fixed factor clock >>>>>> divider/multiplier of the common clock tree binding. >>>>> >>>>> This patch remind me of something : >>>>> http://lists.infradead.org/pipermail/linux-arm-kernel/2012-August/116405.html >>>>> >>>>> Do you have a use case for it, this time? >>>>> >>>>> My version have been hold off waiting for an user for it. >>>>> Will you need it for a driver? >>>>> >>>>> Regards, >>>>> >>>>>> >>>>>> Signed-off-by: Christian Ruppert <christian.ruppert@abilis.com> >>>>>> --- >>>>>> .../bindings/clock/fixed-factor-clkdiv.txt | 24 +++++++++++++++ >>>>>> drivers/clk/clk-fixed-factor.c | 32 ++++++++++++++++++++ >>>>>> include/linux/clk-provider.h | 1 + >>>>>> 3 files changed, 57 insertions(+), 0 deletions(-) >>>>>> create mode 100644 Documentation/devicetree/bindings/clock/fixed-factor-clkdiv.txt >>>>>> >>>>>> diff --git a/Documentation/devicetree/bindings/clock/fixed-factor-clkdiv.txt b/Documentation/devicetree/bindings/clock/fixed-factor-clkdiv.txt >>>>>> new file mode 100644 >>>>>> index 0000000..352bac4 >>>>>> --- /dev/null >>>>>> +++ b/Documentation/devicetree/bindings/clock/fixed-factor-clkdiv.txt >>>>>> @@ -0,0 +1,24 @@ >>>>>> +Device Tree Clock bindings for plat-tb10x >>>>>> + >>>>>> +This binding uses the common clock binding[1]. >>>>>> + >>>>>> +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt >>>>>> + >>>>>> +Required properties: >>>>>> +- compatible : shall be "fixed-factor-clkdiv" >>>>>> +- #clock-cells: from common clock binding; shall be set to 0 >>>>>> +- clocks: shall be the input parent clock phandle for the clock. >>>>>> +- clock-mult: defines the multiplication factor of the output clock frequency >>>>>> + wrt. the input clock frequency. >>>>>> +- clock-div: defines the division factor of the output clock frequency wrt. >>>>>> + the input clock frequency. >>>>>> + >>>>>> +Example: >>>>>> +cpu_clk: clkdiv_cpu { /* CPU clock derived from pll0. 1/2 of pll frequency */ >>>>>> + compatible = "fixed-factor-clkdiv"; >>>>>> + #clock-cells = <0>; >>>>>> + clocks = <&pll0>; >>>>>> + clock-mult = <1>; >>>>>> + clock-div = <2>; >>>>>> + clock-output-names = "cpu_clk"; >>>>>> +}; >>>>>> diff --git a/drivers/clk/clk-fixed-factor.c b/drivers/clk/clk-fixed-factor.c >>>>>> index 1ef271e..85e45f1 100644 >>>>>> --- a/drivers/clk/clk-fixed-factor.c >>>>>> +++ b/drivers/clk/clk-fixed-factor.c >>>>>> @@ -11,6 +11,7 @@ >>>>>> #include <linux/clk-provider.h> >>>>>> #include <linux/slab.h> >>>>>> #include <linux/err.h> >>>>>> +#include <linux/of.h> >>>>>> >>>>>> /* >>>>>> * DOC: basic fixed multiplier and divider clock that cannot gate >>>>>> @@ -96,3 +97,34 @@ struct clk *clk_register_fixed_factor(struct device *dev, const char *name, >>>>>> >>>>>> return clk; >>>>>> } >>>>>> + >>>>>> +#ifdef CONFIG_OF >>>>>> +/** >>>>>> + * of_fixed_factor_clkdiv_setup() - Set up simple fixed factor clock divider >>>>>> + */ >>>>>> +void of_fixed_factor_clkdiv_setup(struct device_node *node) >>>>>> +{ >>>>>> + struct clk *clk; >>>>>> + const char *clk_name = node->name; >>>>>> + const char *parent_name; >>>>>> + u32 mult, div; >>>>>> + >>>>>> + if (of_property_read_u32(node, "clock-mult", &mult)) >>>>>> + return; >>>>>> + >>>>>> + if (of_property_read_u32(node, "clock-div", &div)) >>>>>> + return; >>>>>> + >>>>>> + parent_name = of_clk_get_parent_name(node, 0); >>>>>> + >>>>>> + of_property_read_string(node, "clock-output-names", &clk_name); >>>>>> + >>>>>> + clk = clk_register_fixed_factor(NULL, clk_name, parent_name, 0, >>>>>> + mult, div); >>>>>> + if (!IS_ERR(clk)) >>>>>> + of_clk_add_provider(node, of_clk_src_simple_get, clk); >>>>>> +} >>>>>> +EXPORT_SYMBOL_GPL(of_fixed_factor_clkdiv_setup); >>>>>> +CLK_OF_DECLARE(fixed_clkdiv, "fixed-factor-clkdiv", >>>>>> + of_fixed_factor_clkdiv_setup); >>>>>> +#endif >>>>>> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h >>>>>> index 7f197d7..d4937cf 100644 >>>>>> --- a/include/linux/clk-provider.h >>>>>> +++ b/include/linux/clk-provider.h >>>>>> @@ -184,6 +184,7 @@ struct clk *clk_register_fixed_rate(struct device *dev, const char *name, >>>>>> unsigned long fixed_rate); >>>>>> >>>>>> void of_fixed_clk_setup(struct device_node *np); >>>>>> +void of_fixed_factor_clkdiv_setup(struct device_node *node); >>>>>> >>>>>> /** >>>>>> * struct clk_gate - gating clock >>>>>> >>>>> >>>>> >>>>> -- >>>>> Gregory Clement, Free Electrons >>>>> Kernel, drivers, real-time and embedded Linux >>>>> development, consulting, training and support. >>>>> http://free-electrons.com >>>> >>>> -- >>>> Christian Ruppert , <christian.ruppert@abilis.com> >>>> /| >>>> Tel: +41/(0)22 816 19-42 //| 3, Chemin du Pré-Fleuri >>>> _// | bilis Systems CH-1228 Plan-les-Ouates >>> >> >> >> -- >> Gregory Clement, Free Electrons >> Kernel, drivers, real-time and embedded Linux >> development, consulting, training and support. >> http://free-electrons.com > -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] clk: Add device tree binding to clk-fixed-factor 2013-04-12 9:04 ` Gregory CLEMENT @ 2013-04-12 9:12 ` Christian Ruppert 2013-04-12 9:46 ` Gregory CLEMENT 0 siblings, 1 reply; 12+ messages in thread From: Christian Ruppert @ 2013-04-12 9:12 UTC (permalink / raw) To: Gregory CLEMENT Cc: Mike Turquette, linux-doc, devicetree-discuss, linux-kernel, Rob Herring, Grant Likely, Rob Landley, Vineet Gupta, linux-arm-kernel On Fri, Apr 12, 2013 at 11:04:51AM +0200, Gregory CLEMENT wrote: > On 04/12/2013 08:54 AM, Christian Ruppert wrote: > > On Thu, Apr 11, 2013 at 06:26:07PM +0200, Gregory CLEMENT wrote: > >> On 04/11/2013 11:19 AM, Christian Ruppert wrote: > >>> Hi Gregory, > >>> > >>> Since there doesn't seem to be anyone opposing this feature I just > >>> checked your patch and after adding the CLK_OF_DECLARE macro in the end > >>> it works well for us. Tell me if you would like to rebase and resubmit > >>> your patch or if I should follow up on our own. > >> > >> I can rebase and resubmit my patch, it's pretty straightforward. > >> The problem was the lack of user in the kernel. And by user I mean > >> a driver using these functions. Usually a new function enter in the > >> kernel only if there are users for it inside the kernel so if you plan > >> to submit a driver using this piece of code, then I see no reason > >> to not get it. > > > > If you add the line > > > > CLK_OF_DECLARE(fixed_factor_clk, "fixed-factor-clkdiv", > > Why did you add the div suffix? > > There is nothing in the function which prevent to have "mul" greater than > "div" and hence having a multiplicative factor. > > I think that this suffix is misleading. You are right, let's remove it. > [...] -- Christian Ruppert , <christian.ruppert@abilis.com> /| Tel: +41/(0)22 816 19-42 //| 3, Chemin du Pré-Fleuri _// | bilis Systems CH-1228 Plan-les-Ouates ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] clk: Add device tree binding to clk-fixed-factor 2013-04-12 9:12 ` Christian Ruppert @ 2013-04-12 9:46 ` Gregory CLEMENT 2013-04-12 10:36 ` [PATCH] ARC: [TB10x] Adapt device tree to new compatible string Christian Ruppert 0 siblings, 1 reply; 12+ messages in thread From: Gregory CLEMENT @ 2013-04-12 9:46 UTC (permalink / raw) To: Christian Ruppert Cc: linux-kernel, linux-arm-kernel, Mike Turquette, linux-doc, devicetree-discuss, Rob Herring, Grant Likely, Rob Landley, Vineet Gupta On 04/12/2013 11:12 AM, Christian Ruppert wrote: > On Fri, Apr 12, 2013 at 11:04:51AM +0200, Gregory CLEMENT wrote: >> On 04/12/2013 08:54 AM, Christian Ruppert wrote: >>> On Thu, Apr 11, 2013 at 06:26:07PM +0200, Gregory CLEMENT wrote: >>>> On 04/11/2013 11:19 AM, Christian Ruppert wrote: >>>>> Hi Gregory, >>>>> >>>>> Since there doesn't seem to be anyone opposing this feature I just >>>>> checked your patch and after adding the CLK_OF_DECLARE macro in the end >>>>> it works well for us. Tell me if you would like to rebase and resubmit >>>>> your patch or if I should follow up on our own. >>>> >>>> I can rebase and resubmit my patch, it's pretty straightforward. >>>> The problem was the lack of user in the kernel. And by user I mean >>>> a driver using these functions. Usually a new function enter in the >>>> kernel only if there are users for it inside the kernel so if you plan >>>> to submit a driver using this piece of code, then I see no reason >>>> to not get it. >>> >>> If you add the line >>> >>> CLK_OF_DECLARE(fixed_factor_clk, "fixed-factor-clkdiv", >> >> Why did you add the div suffix? >> >> There is nothing in the function which prevent to have "mul" greater than >> "div" and hence having a multiplicative factor. >> >> I think that this suffix is misleading. > > You are right, let's remove it. Great, I am going to send the patch in a couple of minutes. That means that you will have to ammend your file arch/arc/boot/dts/abilis_tb10x.dtsi and for the clock named cpu_clk and ahb_clk you will have to rename fixed-factor-clkdiv to fixed-factor-clock Thanks, > >> [...] > -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] ARC: [TB10x] Adapt device tree to new compatible string 2013-04-12 9:46 ` Gregory CLEMENT @ 2013-04-12 10:36 ` Christian Ruppert 2013-04-12 10:38 ` Vineet Gupta 0 siblings, 1 reply; 12+ messages in thread From: Christian Ruppert @ 2013-04-12 10:36 UTC (permalink / raw) To: Vineet Gupta, Gregory CLEMENT Cc: Mike Turquette, linux-doc, devicetree-discuss, linux-kernel, Rob Herring, Grant Likely, Rob Landley, Christian Ruppert, linux-arm-kernel The original device tree was written using a slightly different implementation of the fixed-factor-clock device tree binding. The compatible string must be modified in order to be compatible with the new implementation. Signed-off-by: Christian Ruppert <christian.ruppert@abilis.com> --- arch/arc/boot/dts/abilis_tb10x.dtsi | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/arc/boot/dts/abilis_tb10x.dtsi b/arch/arc/boot/dts/abilis_tb10x.dtsi index 6d80d9c..a6139fc 100644 --- a/arch/arc/boot/dts/abilis_tb10x.dtsi +++ b/arch/arc/boot/dts/abilis_tb10x.dtsi @@ -53,13 +53,13 @@ clock-output-names = "pll0"; }; cpu_clk: clkdiv_cpu { - compatible = "fixed-factor-clkdiv"; + compatible = "fixed-factor-clock"; #clock-cells = <0>; clocks = <&pll0>; clock-output-names = "cpu_clk"; }; ahb_clk: clkdiv_ahb { - compatible = "fixed-factor-clkdiv"; + compatible = "fixed-factor-clock"; #clock-cells = <0>; clocks = <&pll0>; clock-output-names = "ahb_clk"; -- 1.7.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] ARC: [TB10x] Adapt device tree to new compatible string 2013-04-12 10:36 ` [PATCH] ARC: [TB10x] Adapt device tree to new compatible string Christian Ruppert @ 2013-04-12 10:38 ` Vineet Gupta 0 siblings, 0 replies; 12+ messages in thread From: Vineet Gupta @ 2013-04-12 10:38 UTC (permalink / raw) To: Christian Ruppert Cc: Gregory CLEMENT, linux-kernel, linux-arm-kernel, Mike Turquette, linux-doc, devicetree-discuss, Rob Herring, Grant Likely, Rob Landley On 04/12/2013 04:06 PM, Christian Ruppert wrote: > The original device tree was written using a slightly different > implementation of the fixed-factor-clock device tree binding. The > compatible string must be modified in order to be compatible with the > new implementation. > > Signed-off-by: Christian Ruppert <christian.ruppert@abilis.com> Applied ! Thx, -Vineet ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2013-04-12 10:38 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-04-10 15:40 [PATCH] clk: Add device tree binding to clk-fixed-factor Christian Ruppert 2013-04-10 15:56 ` Gregory CLEMENT 2013-04-10 16:27 ` Christian Ruppert 2013-04-11 9:19 ` Christian Ruppert 2013-04-11 16:26 ` Gregory CLEMENT 2013-04-12 6:54 ` Christian Ruppert 2013-04-12 7:05 ` Gregory CLEMENT 2013-04-12 9:04 ` Gregory CLEMENT 2013-04-12 9:12 ` Christian Ruppert 2013-04-12 9:46 ` Gregory CLEMENT 2013-04-12 10:36 ` [PATCH] ARC: [TB10x] Adapt device tree to new compatible string Christian Ruppert 2013-04-12 10:38 ` Vineet Gupta
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).