All of lore.kernel.org
 help / color / mirror / Atom feed
From: gregory.clement@free-electrons.com (Gregory CLEMENT)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] clk: Add device tree binding to clk-fixed-factor
Date: Thu, 11 Apr 2013 18:26:07 +0200	[thread overview]
Message-ID: <5166E41F.7060100@free-electrons.com> (raw)
In-Reply-To: <20130411091952.GF8400@ab42.lan>

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

WARNING: multiple messages have this Message-ID (diff)
From: Gregory CLEMENT <gregory.clement@free-electrons.com>
To: Christian Ruppert <christian.ruppert@abilis.com>
Cc: linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	Mike Turquette <mturquette@linaro.org>,
	linux-doc@vger.kernel.org, devicetree-discuss@lists.ozlabs.org,
	Rob Herring <rob.herring@calxeda.com>,
	Grant Likely <grant.likely@secretlab.ca>,
	Rob Landley <rob@landley.net>,
	Vineet Gupta <Vineet.Gupta1@synopsys.com>
Subject: Re: [PATCH] clk: Add device tree binding to clk-fixed-factor
Date: Thu, 11 Apr 2013 18:26:07 +0200	[thread overview]
Message-ID: <5166E41F.7060100@free-electrons.com> (raw)
In-Reply-To: <20130411091952.GF8400@ab42.lan>

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

  reply	other threads:[~2013-04-11 16:26 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-10 15:40 [PATCH] clk: Add device tree binding to clk-fixed-factor Christian Ruppert
2013-04-10 15:40 ` Christian Ruppert
2013-04-10 15:40 ` Christian Ruppert
2013-04-10 15:56 ` Gregory CLEMENT
2013-04-10 15:56   ` Gregory CLEMENT
2013-04-10 16:27   ` Christian Ruppert
2013-04-10 16:27     ` Christian Ruppert
2013-04-11  9:19     ` Christian Ruppert
2013-04-11  9:19       ` Christian Ruppert
2013-04-11 16:26       ` Gregory CLEMENT [this message]
2013-04-11 16:26         ` Gregory CLEMENT
2013-04-12  6:54         ` Christian Ruppert
2013-04-12  6:54           ` Christian Ruppert
2013-04-12  7:05           ` Gregory CLEMENT
2013-04-12  7:05             ` Gregory CLEMENT
2013-04-12  9:04           ` Gregory CLEMENT
2013-04-12  9:04             ` Gregory CLEMENT
2013-04-12  9:12             ` Christian Ruppert
2013-04-12  9:12               ` Christian Ruppert
2013-04-12  9:12               ` Christian Ruppert
2013-04-12  9:46               ` Gregory CLEMENT
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:36                   ` Christian Ruppert
2013-04-12 10:36                   ` Christian Ruppert
2013-04-12 10:38                   ` Vineet Gupta
2013-04-12 10:38                     ` Vineet Gupta
2013-04-12 10:38                     ` Vineet Gupta

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5166E41F.7060100@free-electrons.com \
    --to=gregory.clement@free-electrons.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.