All of lore.kernel.org
 help / color / mirror / Atom feed
From: Shawn Guo <shawn.guo@freescale.com>
To: Rob Herring <robherring2@gmail.com>
Cc: Mike Turquette <mturquette@linaro.org>,
	Grant Likely <grant.likely@secretlab.ca>,
	"arm@kernel.org" <arm@kernel.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [GIT PULL] DT clk binding support
Date: Sun, 20 May 2012 11:06:54 +0800	[thread overview]
Message-ID: <20120520030653.GB5810@S2100-06.ap.freescale.net> (raw)
In-Reply-To: <4FB80F32.5090309@gmail.com>

On Sat, May 19, 2012 at 04:22:58PM -0500, Rob Herring wrote:
> Grant Likely (2):
>       clk: add DT clock binding support

I just checked your branch and found that a couple of comments that
I put on this patch haven't get addressed.  The most notable one would
be the "clocks" property of clock consumers.

+==Clock consumers==
+
+Required properties:
+clocks:                List of phandle and clock specifier pairs, one pair
+               for each clock input to the device.  Note: if the
+               clock provider specifies '0' for #clock-cells, then
+               only the phandle portion of the pair will appear.
+
+Optional properties:
+clock-names:   List of clock input name strings sorted in the same
+               order as the clocks property.  Consumers drivers
+               will use clock-names to match clock input names
+               with clocks specifiers.

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.

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.

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.

-- 
Regards,
Shawn


WARNING: multiple messages have this Message-ID (diff)
From: shawn.guo@freescale.com (Shawn Guo)
To: linux-arm-kernel@lists.infradead.org
Subject: [GIT PULL] DT clk binding support
Date: Sun, 20 May 2012 11:06:54 +0800	[thread overview]
Message-ID: <20120520030653.GB5810@S2100-06.ap.freescale.net> (raw)
In-Reply-To: <4FB80F32.5090309@gmail.com>

On Sat, May 19, 2012 at 04:22:58PM -0500, Rob Herring wrote:
> Grant Likely (2):
>       clk: add DT clock binding support

I just checked your branch and found that a couple of comments that
I put on this patch haven't get addressed.  The most notable one would
be the "clocks" property of clock consumers.

+==Clock consumers==
+
+Required properties:
+clocks:                List of phandle and clock specifier pairs, one pair
+               for each clock input to the device.  Note: if the
+               clock provider specifies '0' for #clock-cells, then
+               only the phandle portion of the pair will appear.
+
+Optional properties:
+clock-names:   List of clock input name strings sorted in the same
+               order as the clocks property.  Consumers drivers
+               will use clock-names to match clock input names
+               with clocks specifiers.

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.

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.

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.

-- 
Regards,
Shawn

  reply	other threads:[~2012-05-20  2:47 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-19 21:22 [GIT PULL] DT clk binding support Rob Herring
2012-05-19 21:22 ` Rob Herring
2012-05-20  3:06 ` Shawn Guo [this message]
2012-05-20  3:06   ` Shawn Guo
2012-05-21  2:18   ` Rob Herring
2012-05-21  2:18     ` Rob Herring
2012-05-21  6:49     ` Shawn Guo
2012-05-21  6:49       ` Shawn Guo
2012-05-21 18:30       ` Rob Herring
2012-05-21 18:30         ` Rob Herring
2012-05-21 23:26         ` Shawn Guo
2012-05-21 23:26           ` Shawn Guo
2012-05-21 23:52           ` Rob Herring
2012-05-21 23:52             ` Rob Herring
2012-05-22  2:15             ` Shawn Guo
2012-05-22  2:15               ` Shawn Guo
2012-05-22  4:17               ` Stephen Boyd
2012-05-22  4:17                 ` Stephen Boyd
2012-05-22 13:52                 ` Rob Herring
2012-05-22 13:52                   ` Rob Herring
2012-05-23  1:38                   ` Saravana Kannan
2012-05-23  1:38                     ` Saravana Kannan
2012-05-23 13:59                     ` Rob Herring
2012-05-23 13:59                       ` Rob Herring
2012-05-24 21:16                       ` Saravana Kannan
2012-05-24 21:16                         ` Saravana Kannan
2012-05-24 21:54                         ` Rob Herring
2012-05-24 21:54                           ` Rob Herring
2012-05-25  3:33                           ` Saravana Kannan
2012-05-25  3:33                             ` Saravana Kannan
2012-06-01 13:21                             ` Rob Herring
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=20120520030653.GB5810@S2100-06.ap.freescale.net \
    --to=shawn.guo@freescale.com \
    --cc=arm@kernel.org \
    --cc=grant.likely@secretlab.ca \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mturquette@linaro.org \
    --cc=robherring2@gmail.com \
    /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.