From: robherring2@gmail.com (Rob Herring)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 4/7] dt/clock: Add handling for fixed clocks and a clock node setup iterator
Date: Sat, 14 Apr 2012 22:04:26 -0500 [thread overview]
Message-ID: <4F8A3ABA.2080201@gmail.com> (raw)
In-Reply-To: <20120409232723.GD18692@S2101-09.ap.freescale.net>
On 04/09/2012 06:27 PM, Shawn Guo wrote:
> On Mon, Apr 09, 2012 at 09:18:27AM -0500, Rob Herring wrote:
> ...
>> You still haven't given any benefits of supporting multiple clocks. It's
>> slightly fewer dts lines, but not really anything else.
>>
> What's more important than fewer dts lines is fewer nodes. Isn't it
> the whole point of #clock-cells? In the real imx example I gave, with
> #clock-cells support, I can have only one node to represent 3 fixed
> clocks.
So you want to have generic bindings for every clock which implies a
bunch of nodes if not a node per clock anyway, but then object to 3
nodes for fixed clocks.
I don't think I've ever seen a chip with more than 3 clock inputs
anyway. There are cases with additional clocks like a CMOS sensor or
audio chip which provides a fixed input clock, but those clocks should
reside with the nodes that generate them.
> ...
>> I don't think people are going to define clocks generically in DT at the
>> mux, clk gate and divider level anyway. If you only have a few clocks
>> then you may, and 1 clock output per node is probably okay. If you have
>> hundreds of clocks then you probably won't, and will have a SOC specific
>> binding anyway.
>>
> Why?
>
> Take a look at clk-imx6q.c[1], you will find except pll and pfd, all
> the imx6q clocks are represented as gate, divider and mux, and we
> should not need a SoC specific binding for them.
>
I'm talking about the bindings and you are pointing me to an
implementation with no bindings. It's 2 different things. The
implementation can fully be the generic clk implementations, but the
clock bindings can still be either a node per clock or a monolithic
clock module node. There is no fixed rule here and there should not be.
We can each chose what works best for us.
I see a couple of examples that your clocks are still SoC specific
despite your claims.
Your clock gates may reuse clk_gate struct, but they still have custom
ops to handle the 2-bit field. Obviously, you decided not to merge 2-bit
field support into the existing clk gate code (the correct decision
IMHO). I made a similar decision.
For practically every clock, you need to set the spinlock to
imx_ccm_lock. How are you going to know which clocks to set this lock
for with a "generic" binding? You have to distinguish those from IPU
clocks for example.
Rob
WARNING: multiple messages have this Message-ID (diff)
From: Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
To: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
Mike Turquette
<mturquette-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Subject: Re: [PATCH 4/7] dt/clock: Add handling for fixed clocks and a clock node setup iterator
Date: Sat, 14 Apr 2012 22:04:26 -0500 [thread overview]
Message-ID: <4F8A3ABA.2080201@gmail.com> (raw)
In-Reply-To: <20120409232723.GD18692-rvtDTF3kK1ictlrPMvKcciBecyulp+rMXqFh9Ls21Oc@public.gmane.org>
On 04/09/2012 06:27 PM, Shawn Guo wrote:
> On Mon, Apr 09, 2012 at 09:18:27AM -0500, Rob Herring wrote:
> ...
>> You still haven't given any benefits of supporting multiple clocks. It's
>> slightly fewer dts lines, but not really anything else.
>>
> What's more important than fewer dts lines is fewer nodes. Isn't it
> the whole point of #clock-cells? In the real imx example I gave, with
> #clock-cells support, I can have only one node to represent 3 fixed
> clocks.
So you want to have generic bindings for every clock which implies a
bunch of nodes if not a node per clock anyway, but then object to 3
nodes for fixed clocks.
I don't think I've ever seen a chip with more than 3 clock inputs
anyway. There are cases with additional clocks like a CMOS sensor or
audio chip which provides a fixed input clock, but those clocks should
reside with the nodes that generate them.
> ...
>> I don't think people are going to define clocks generically in DT at the
>> mux, clk gate and divider level anyway. If you only have a few clocks
>> then you may, and 1 clock output per node is probably okay. If you have
>> hundreds of clocks then you probably won't, and will have a SOC specific
>> binding anyway.
>>
> Why?
>
> Take a look at clk-imx6q.c[1], you will find except pll and pfd, all
> the imx6q clocks are represented as gate, divider and mux, and we
> should not need a SoC specific binding for them.
>
I'm talking about the bindings and you are pointing me to an
implementation with no bindings. It's 2 different things. The
implementation can fully be the generic clk implementations, but the
clock bindings can still be either a node per clock or a monolithic
clock module node. There is no fixed rule here and there should not be.
We can each chose what works best for us.
I see a couple of examples that your clocks are still SoC specific
despite your claims.
Your clock gates may reuse clk_gate struct, but they still have custom
ops to handle the 2-bit field. Obviously, you decided not to merge 2-bit
field support into the existing clk gate code (the correct decision
IMHO). I made a similar decision.
For practically every clock, you need to set the spinlock to
imx_ccm_lock. How are you going to know which clocks to set this lock
for with a "generic" binding? You have to distinguish those from IPU
clocks for example.
Rob
next prev parent reply other threads:[~2012-04-15 3:04 UTC|newest]
Thread overview: 62+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-03-13 23:22 [PATCH 0/7] Highbank clock support using DT Rob Herring
2012-03-13 23:22 ` Rob Herring
2012-03-13 23:22 ` [PATCH 1/7] clk: fix orphan list iterator to be safe Rob Herring
2012-03-13 23:22 ` Rob Herring
2012-03-14 2:10 ` Turquette, Mike
2012-03-14 2:10 ` Turquette, Mike
2012-03-13 23:22 ` [PATCH 2/7] of: add clock providers Rob Herring
2012-03-13 23:22 ` Rob Herring
2012-03-14 7:07 ` Thierry Reding
2012-03-14 7:07 ` Thierry Reding
2012-03-14 7:55 ` Shawn Guo
2012-03-14 7:55 ` Shawn Guo
2012-04-07 4:18 ` Grant Likely
2012-04-07 4:18 ` Grant Likely
2012-04-07 19:04 ` Rob Herring
2012-04-07 19:04 ` Rob Herring
2012-04-09 11:55 ` Shawn Guo
2012-04-09 11:55 ` Shawn Guo
2012-04-09 13:52 ` Rob Herring
2012-04-09 13:52 ` Rob Herring
2012-04-09 14:13 ` Shawn Guo
2012-04-09 14:13 ` Shawn Guo
2012-04-09 14:34 ` Rob Herring
2012-04-09 14:34 ` Rob Herring
2012-04-09 23:42 ` Shawn Guo
2012-04-09 23:42 ` Shawn Guo
2012-03-13 23:22 ` [PATCH 3/7] of: Add of_property_match_string() to find index into a string list Rob Herring
2012-03-13 23:22 ` Rob Herring
2012-04-07 4:22 ` Grant Likely
2012-04-07 4:22 ` Grant Likely
2012-03-13 23:22 ` [PATCH 4/7] dt/clock: Add handling for fixed clocks and a clock node setup iterator Rob Herring
2012-03-13 23:22 ` Rob Herring
2012-03-14 7:59 ` Shawn Guo
2012-03-14 7:59 ` Shawn Guo
2012-03-14 13:26 ` Rob Herring
2012-03-14 13:26 ` Rob Herring
2012-03-14 13:45 ` Shawn Guo
2012-03-14 13:45 ` Shawn Guo
2012-04-08 14:48 ` Rob Herring
2012-04-08 14:48 ` Rob Herring
2012-04-09 8:49 ` Shawn Guo
2012-04-09 8:49 ` Shawn Guo
2012-04-09 14:18 ` Rob Herring
2012-04-09 14:18 ` Rob Herring
2012-04-09 23:27 ` Shawn Guo
2012-04-09 23:27 ` Shawn Guo
2012-04-15 3:04 ` Rob Herring [this message]
2012-04-15 3:04 ` Rob Herring
2012-04-15 7:01 ` Shawn Guo
2012-04-15 7:01 ` Shawn Guo
2012-03-13 23:22 ` [PATCH 5/7] dt/clock: add a simple provider get function Rob Herring
2012-03-13 23:22 ` Rob Herring
2012-04-07 4:26 ` Grant Likely
2012-04-07 4:26 ` Grant Likely
2012-03-13 23:22 ` [PATCH 6/7] dt/clock: add function to get parent clock name Rob Herring
2012-03-13 23:22 ` Rob Herring
2012-03-13 23:22 ` [PATCH 7/7] clk: add highbank clock support Rob Herring
2012-03-13 23:22 ` Rob Herring
2012-04-10 2:06 ` Shawn Guo
2012-04-10 2:06 ` Shawn Guo
2012-04-10 13:17 ` Rob Herring
2012-04-10 13:17 ` 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=4F8A3ABA.2080201@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 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.