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: Mon, 09 Apr 2012 09:18:27 -0500 [thread overview]
Message-ID: <4F82EFB3.80402@gmail.com> (raw)
In-Reply-To: <20120409084930.GA18692@S2101-09.ap.freescale.net>
On 04/09/2012 03:49 AM, Shawn Guo wrote:
> On Sun, Apr 08, 2012 at 09:48:27AM -0500, Rob Herring wrote:
> ...
>> So I started implementing support for multiple outputs, but ran into a
>> complication. If you have multiple clocks on a node, then you have to
>> have a clk_src_get function to translate the clock cell value to a
>> struct clk.
>
> So this is how clk_src_get looks like.
>
> struct clk *clk_src_get(struct of_phandle_args *a, void *data)
> {
> struct clk **clks = data;
> return clks[a->args[0]];
> }
>
>> This would also require allocating an array of struct clk's
>> to do the lookup.
>
> And this how allocating looks like.
>
> clks = kzalloc(sizeof(*clks) * num, GFP_KERNEL);
> if (!clks)
> return -ENOMEM;
>
> Seriously, not a big complication, right?
I didn't say it can't be done. I'm saying I don't see the benefit to
supporting it vs. the added code to iterate over the array, handle
errors in the middle, and added matching function.
You still haven't given any benefits of supporting multiple clocks. It's
slightly fewer dts lines, but not really anything else.
>
>> This is all doable, but I don't see the benefit over
>> having a single node per fixed clock. We're not likely to have *lots* of
>> fixed clocks. I think we should leave fixed-clock defined as a single
>> output.
>
> The problem is there is nothing specific to fixed-clock. If you have
> some reason to not support blob node for fixed-clock, the reason will
> apply on clk_gate, clk_divider and clk_mux too. Then, which clocks
> will support the #clock-cells in the binding document?
>
But fixed-clock is a specific binding and each binding can define
#clock-cells however they want. New bindings can define whatever they want.
If you did a clk mux binding, you're not going to have multiple lists of
mux selections for multiple outputs, so we're going to effectively
limited there to 1 output as well.
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.
Rob
> So to me, you need to either have it implemented or remove it from the
> binding document completely.
>
> Regards,
> Shawn
>
>> If you really see the benefit, you can add a new binding
>> "multiple-fixed-clocks" or something platform specific.
>>
next prev parent reply other threads:[~2012-04-09 14:18 UTC|newest]
Thread overview: 31+ 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 ` [PATCH 1/7] clk: fix orphan list iterator to be safe Rob Herring
2012-03-14 2:10 ` Turquette, Mike
2012-03-13 23:22 ` [PATCH 2/7] of: add clock providers Rob Herring
2012-03-14 7:07 ` Thierry Reding
2012-03-14 7:55 ` Shawn Guo
2012-04-07 4:18 ` Grant Likely
2012-04-07 19:04 ` Rob Herring
2012-04-09 11:55 ` Shawn Guo
2012-04-09 13:52 ` Rob Herring
2012-04-09 14:13 ` Shawn Guo
2012-04-09 14:34 ` Rob Herring
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-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-14 7:59 ` Shawn Guo
2012-03-14 13:26 ` Rob Herring
2012-03-14 13:45 ` Shawn Guo
2012-04-08 14:48 ` Rob Herring
2012-04-09 8:49 ` Shawn Guo
2012-04-09 14:18 ` Rob Herring [this message]
2012-04-09 23:27 ` Shawn Guo
2012-04-15 3:04 ` Rob Herring
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-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 ` [PATCH 7/7] clk: add highbank clock support Rob Herring
2012-04-10 2:06 ` Shawn Guo
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=4F82EFB3.80402@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).