From: s.nawrocki@samsung•com (Sylwester Nawrocki)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH RFC v1 3/3] clk: Add handling of clk parent and rate assigned from DT
Date: Mon, 24 Feb 2014 19:11:18 +0100 [thread overview]
Message-ID: <530B8B46.6060003@samsung.com> (raw)
In-Reply-To: <20140224004826.22529.87768@quantum>
On 24/02/14 01:48, Mike Turquette wrote:
> Quoting Sylwester Nawrocki (2014-02-21 02:38:21)
>> > On 20/02/14 15:09, Grant Likely wrote:
>> > [...]
>>>>> > >> > diff --git a/Documentation/devicetree/bindings/clock/clock-bindings.txt b/Documentation/devicetree/bindings/clock/clock-bindings.txt
>>>>> > >> > index 7c52c29..d618498 100644
>>>>> > >> > --- a/Documentation/devicetree/bindings/clock/clock-bindings.txt
>>>>> > >> > +++ b/Documentation/devicetree/bindings/clock/clock-bindings.txt
>>>>> > >> > @@ -115,3 +115,27 @@ clock signal, and a UART.
>>>>> > >> > ("pll" and "pll-switched").
>>>>> > >> > * The UART has its baud clock connected the external oscillator and its
>>>>> > >> > register clock connected to the PLL clock (the "pll-switched" signal)
>>>>> > >> > +
>>>>> > >> > +==Static initial configuration of clock parent and clock frequency==
>>>>> > >> > +
>>>>> > >> > +Some platforms require static configuration of (parts of) the clock controller
>>>>> > >> > +often determined by the board design. Such a configuration can be specified in
>>>>> > >> > +a clock consumer node through [clk-name]-clk-parent and [clk-name]-clk-rate DT
>>>>> > >> > +properties. The former should contain phandle and clock specifier of the parent
>>>>> > >> > +clock, the latter the required clock's frequency value (one cell). "clk-name"
>>>>> > >> > +should be listed in the clock-names property and a phandle and a clock specifier
>>>>> > >> > +pair corresponding to it should be present in the clocks property.
>>>>> > >> > +
>>>>> > >> > + uart at a000 {
>>>>> > >> > + compatible = "fsl,imx-uart";
>>>>> > >> > + reg = <0xa000 0x1000>;
>>>>> > >> > + ...
>>>>> > >> > + clocks = <&clkcon 0>, <&clkcon 3>;
>>>>> > >> > + clock-names = "baud", "mux";
>>>>> > >> > +
>>>>> > >> > + mux-clk-parent = <&pll 1>;
>>>>> > >> > + baud-clk-rate = <460800>;
>>> > >
>>> > > This mixes patterns for references to clocks. Plus it requires composing
>>> > > property names which is a little painful. I'd rather see a list of
>>> > > tuples to match the existing pattern already in use
>>> > >
>>> > > clocks = <&clkcon 0>, <&clkcon 3>;
>>> > > clock-names = "baud", "mux";
>>> > > clock-parents = <0> <&pll 1>;
>>> > > clock-rates = <0> <460800>;
>> >
>> > Thank you for the review. This looks much better to me. My bad, I wasn't
>> > aware 0 can be used to denote an empty phandle like this. ePAPR seems not
>> > to be specifying exact meaning of the 'phandle' property values, except
>> > they be unique. Anyway, it seems to be clearly documented within the
>> > __of_parse_phandle_with_args() function.
>> >
>> > I'll try this modified binding instead, presumably it would be useful to
>> > have a variant of __of_parse_phandle_with_args() function which would
>> > accept a context data containing result of previous call within an iteration,
>> > similarly as of_property_next_string() is written. So we don't iterate
>> > from beginning of the list with each __of_parse_phandle_with_args() call.
>> > But it's an optimization issue that could be considered separately I guess.
>
> I was always partial to the regulator style of blahblah-supply but
> Grant's suggestion is much cleaner with respect to the rest of the clock
> binding.
I don't have a strong opinion, I'm slightly inclined towards Grant's suggestion,
which doesn't have a problem of limiting effective clk name to 21 characters.
Also DT property names with underscores coming from the clock names are a bit
incorrect and it might be not immediately obvious which part of name is a
canonical property's name and which is clock's name. Let's consider property
names like:
mux-clk-parent, divider-clk-rate, sclk_mmc0-clk-parent, sclk_uart_baud0-clk-parent,
etc.
> I guess it will be a bit ugly if a very long array is needed with a
> sparse attribute. E.g. 20 clocks specified in the 'clocks' property and
> only a single clock needs to have its parent or rate specified.
It was also concerning me, but this inconvenience could be mitigated by
reordering content of clocks/clock-names properties so that the clocks for
which parents and/or rates are being assigned are first on the list.
Then number of padding zeros is minimized.
> Is there a reason not to support both methods?
--
Regards,
Sylwester
next prev parent reply other threads:[~2014-02-24 18:11 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-02-19 16:58 [PATCH RFC v1 0/3] clk: Support for DT assigned clk parent and rate Sylwester Nawrocki
[not found] ` < 1392829124-25705-4-git-send-email-s.nawrocki@samsung.com>
2014-02-19 16:58 ` [PATCH RFC v1 1/3] clk: Add function to parse an arbitrary clocks list property Sylwester Nawrocki
2014-02-24 0:43 ` Mike Turquette
2014-02-24 10:43 ` Sylwester Nawrocki
2014-02-19 16:58 ` [PATCH RFC v1 2/3] of: Add definition of maximum length of a property name Sylwester Nawrocki
2014-02-19 21:42 ` Rob Herring
2014-02-20 15:02 ` Sylwester Nawrocki
2014-02-19 16:58 ` [PATCH RFC v1 3/3] clk: Add handling of clk parent and rate assigned from DT Sylwester Nawrocki
2014-02-20 14:09 ` Grant Likely
2014-02-21 10:38 ` Sylwester Nawrocki
2014-02-24 0:48 ` Mike Turquette
2014-02-24 18:11 ` Sylwester Nawrocki [this message]
2014-03-01 21:13 ` Grant Likely
2014-03-01 21:11 ` Grant Likely
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=530B8B46.6060003@samsung.com \
--to=s.nawrocki@samsung$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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