linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: robherring2@gmail.com (Rob Herring)
To: linux-arm-kernel@lists.infradead.org
Subject: [GIT PULL] DT clk binding support
Date: Mon, 21 May 2012 13:30:59 -0500	[thread overview]
Message-ID: <4FBA89E3.7010106@gmail.com> (raw)
In-Reply-To: <20120521064901.GE8140@S2101-09.ap.freescale.net>

On 05/21/2012 01:49 AM, Shawn Guo wrote:
> On Sun, May 20, 2012 at 09:18:15PM -0500, Rob Herring wrote:
>>> As I said, any clock in the clock tree except root clock is not only
>>> a clock provider but also a consumer.  If you define "clocks" as a
>>> required property for clock consumers, you are essentially asking users
>>> to either define the whole clock tree in the device tree or stay away
>>> from device tree completely.
>>
>> So what are you proposing that a clock consumer have? The very
>> definition of a clock consumer is that it has a clocks property.
>>
> To support the cases that the clock tree is defined by clock driver
> and device tree together, the "clocks" property could be reasonably
> absent in case the parent clock (provider) is being defined in driver
> than DT.  And for such clocks, the "clock-names" than "clocks" should
> be required to find the clock provider/parent.
> 
> I do not like the idea to look for clock with name too much, but I
> do not see other way around to support those platforms that have clock
> tree definition split in clock driver and device tree. 
> 
>>>
>>> Are you sure this is the right thing to do?  If I remember correctly,
>>> Grant's position is it should be pretty reasonable to have most of
>>> the clock tree defined in clock driver and only define those leaf
>>> clocks which are very likely to become the clock providers for other
>>> peripherals.
>>
>> The minimum is you have to have a provider and consumer. It may be a
>> single provider that provides all clocks for a chip. If you don't want a
>> provider, then just define a clock-frequency property.
>>>
>>> Let me put a terrible example here.  Since clock tree is actually SoC
>>> specific, I can reasonably choose to define the entire imx6q clock tree
>>> and all the clk lookups for imx6q peripherals in clk-imx6q driver.
>>> On imx6q-sabrelite board, the audio codec sgtl5000 uses cko (an imx6q
>>> clock available on pad) as the clock source.  That said, I need a board
>>> specific clk lookup here, which should be the best user of clock DT
>>> bindings.  But sadly, with the current bindings, I can not give the
>>> required "clocks" property for sgtl5000 node.
>>
>> I don't understand your example. For the sgtl5000 on the sabrelite, you
>> would provide a phandle and cell entry that is interpreted as the cko
>> pin.
> 
> With the bindings here, I need something like below in device tree to
> replace the clk lookup registration that is currently done in imx6q
> sabrelite specific setup code.  However the problem here is I have cko
> defined in clock driver, and thus I can not give phandle to cko in
> device tree.  What I suggest is for such cases, we could require
> clock-names = "cko" than clocks = <&cko>, and of_clk_get() should also
> be able to find the clock with looking for the clk name.
> 
> imx6q-sabrelite.dts:
> 
> 	codec: sgtl5000 at 0a {
> 		compatible = "fsl,sgtl5000";
> 		reg = <0x0a>;
> 		clocks = <&cko>;
> 	};
> 
> mach-imx6q.c, imx6q_sabrelite_cko1_setup(): 
> 
>         cko1 = clk_get_sys(NULL, "cko1");
>         clk_register_clkdev(cko1, NULL, "0-000a");
> 

What!? This is a terrible abuse/hack of the clock binding and is in no
way what was intended. You cannot use half of the clock binding. You
have to have a provider. The primary binding is a phandle reference.
clock-names is just auxiliary data.

Rob

  reply	other threads:[~2012-05-21 18:30 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-19 21:22 [GIT PULL] DT clk binding support Rob Herring
2012-05-20  3:06 ` Shawn Guo
2012-05-21  2:18   ` Rob Herring
2012-05-21  6:49     ` Shawn Guo
2012-05-21 18:30       ` Rob Herring [this message]
2012-05-21 23:26         ` Shawn Guo
2012-05-21 23:52           ` Rob Herring
2012-05-22  2:15             ` Shawn Guo
2012-05-22  4:17               ` Stephen Boyd
2012-05-22 13:52                 ` Rob Herring
2012-05-23  1:38                   ` Saravana Kannan
2012-05-23 13:59                     ` Rob Herring
2012-05-24 21:16                       ` Saravana Kannan
2012-05-24 21:54                         ` Rob Herring
2012-05-25  3:33                           ` Saravana Kannan
2012-06-01 13:21                             ` Rob Herring

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=4FBA89E3.7010106@gmail.com \
    --to=robherring2@gmail.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 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).