From: Sekhar Nori <nsekhar@ti.com>
To: David Lechner <david@lechnology.com>, Rob Herring <robh@kernel.org>
Cc: linux-clk <linux-clk@vger.kernel.org>,
"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
<devicetree@vger.kernel.org>,
"moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE"
<linux-arm-kernel@lists.infradead.org>,
Michael Turquette <mturquette@baylibre.com>,
Stephen Boyd <sboyd@codeaurora.org>,
Mark Rutland <mark.rutland@arm.com>,
Kevin Hilman <khilman@kernel.org>,
Bartosz Golaszewski <bgolaszewski@baylibre.com>,
Adam Ford <aford173@gmail.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v6 01/41] dt-bindings: clock: Add new bindings for TI Davinci PLL clocks
Date: Wed, 31 Jan 2018 10:28:39 +0530 [thread overview]
Message-ID: <a31b79a0-cf09-8c59-5b14-33daf6c1dccb@ti.com> (raw)
In-Reply-To: <7064a0d6-0969-81a0-00af-84cca339b813@lechnology.com>
On Wednesday 31 January 2018 12:16 AM, David Lechner wrote:
> On 01/30/2018 08:50 AM, Rob Herring wrote:
>> On Mon, Jan 29, 2018 at 3:14 PM, David Lechner <david@lechnology.com>
>> wrote:
>>> On 01/29/2018 01:53 PM, Rob Herring wrote:
>>>>
>>>> On Sat, Jan 20, 2018 at 11:13:40AM -0600, David Lechner wrote:
>>>>>
>>>>> This adds a new binding for the PLL IP blocks in the mach-davinci
>>>>> family of processors. Currently, only da850 has device tree support
>>>>> but these bindings can also work for other SoCs in this family just
>>>>> by adding new compatible strings.
>>>>>
>>>>> Note: Although these PLL controllers are very similar to the TI
>>>>> Keystone
>>>>> SoCs, we are not re-using those bindings. The Keystone bindings use a
>>>>> legacy one-node-per-clock binding. Furthermore, the mach-davinici SoCs
>>>>> have a slightly different PLL register layout and a number of quirks
>>>>> that can't be handled by the existing bindings, so the keystone
>>>>> bindings
>>>>> could not be used as-is anyway.
>>>>>
>>>>> Signed-off-by: David Lechner <david@lechnology.com>
>>>>> ---
>>>>>
>>>>> v6 changes:
>>>>> - Added clock-names property
>>>>> - Added ti,clkmode-square-wave property
>>>>> - Added pllout child node
>>>>> - Added obsclk child node
>>>>> - Expanded examples
>>>>>
>>>>> .../devicetree/bindings/clock/ti/davinci/pll.txt | 96
>>>>> ++++++++++++++++++++++
>>>>> 1 file changed, 96 insertions(+)
>>>>> create mode 100644
>>>>> Documentation/devicetree/bindings/clock/ti/davinci/pll.txt
>>>>>
>>>>> diff --git
>>>>> a/Documentation/devicetree/bindings/clock/ti/davinci/pll.txt
>>>>> b/Documentation/devicetree/bindings/clock/ti/davinci/pll.txt
>>>>> new file mode 100644
>>>>> index 0000000..36998e1
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/clock/ti/davinci/pll.txt
>>>>> @@ -0,0 +1,96 @@
>>>>> +Binding for TI DaVinci PLL Controllers
>>>>> +
>>>>> +The PLL provides clocks to most of the components on the SoC. In
>>>>> addition
>>>>> +to the PLL itself, this controller also contains bypasses, gates,
>>>>> dividers,
>>>>> +an multiplexers for various clock signals.
>>>>> +
>>>>> +Required properties:
>>>>> +- compatible: shall be one of:
>>>>> + - "ti,da850-pll0" for PLL0 on DA850/OMAP-L138/AM18XX
>>>>> + - "ti,da850-pll1" for PLL1 on DA850/OMAP-L138/AM18XX
>>>>> +- reg: physical base address and size of the controller's register
>>>>> area.
>>>>> +- clocks: phandles corresponding to the clock names
>>>>> +- clock-names: names of the clock sources - depends on compatible
>>>>> string
>>>>> + - for "ti,da850-pll0", shall be "clksrc", "extclksrc"
>>>>> + - for "ti,da850-pll1", shall be "clksrc"
>>>>> +
>>>>> +Optional properties:
>>>>> +- ti,clkmode-square-wave: Indicates that the the board is supplying a
>>>>> square
>>>>> + wave input on the OSCIN pin instead of using a crystal
>>>>> oscillator.
>>>>> + This property is only valid when compatible = "ti,da850-pll0".
>>>>> +
>>>>> +
>>>>> +Optional child nodes:
>>>>> +
>>>>> +pllout
>>>>> + Describes the main PLL clock output (before POSTDIV). The node
>>>>> name must
>>>>> + be "pllout".
>>>>> +
>>>>> + Required properties:
>>>>> + - #clock-cells: shall be 0
>>>>> +
>>>>> +sysclk
>>>>> + Describes the PLLDIVn divider clocks that provide the SYSCLKn
>>>>> clock
>>>>> + domains. The node name must be "sysclk". Consumers of this
>>>>> node
>>>>> should
>>>>> + use "n" in "SYSCLKn" as the index parameter for the clock
>>>>> cell.
>>>>> +
>>>>> + Required properties:
>>>>> + - #clock-cells: shall be 1
>>>>> +
>>>>> +auxclk
>>>>> + Describes the AUXCLK output of the PLL. The node name must be
>>>>> "auxclk".
>>>>> + This child node is only valid when compatible =
>>>>> "ti,da850-pll0".
>>>>> +
>>>>> + Required properties:
>>>>> + - #clock-cells: shall be 0
>>>>> +
>>>>> +obsclk
>>>>> + Describes the OBSCLK output of the PLL. The node name must be
>>>>> "obsclk".
>>>>> +
>>>>> + Required properties:
>>>>> + - #clock-cells: shall be 0
>>>>
>>>>
>>>> So why have all these child nodes vs. just defining a single number
>>>> space of clock ids?
>>>>
>>>> Rob
>>>>
>>>
>>> I think that it makes the bindings more self-documenting. Not all
>>> PLLs have
>>> all of possible types of output clocks, so the presence or absence of a
>>> child node indicates if a PLL actually has that output or not.
>>
>> Doesn't the compatible string do that?
>
> Sure.
>
>>
>>> It is also complicated by the fact that one of the child nodes (sysclk)
>>> is already an array of clocks.
>>>
>>> To do what you are suggesting might look something like this...
>>>
>>> ---
>>>
>>> Required properties:
>>> - compatible: shall be one of:
>>> - "ti,da850-pll0" for PLL0 on DA850/OMAP-L138/AM18XX
>>> - "ti,da850-pll1" for PLL1 on DA850/OMAP-L138/AM18XX
>>> - reg: physical base address and size of the controller's register area.
>>> - clocks: phandles corresponding to the clock names
>>> - clock-names: names of the clock sources - depends on compatible string
>>> - for "ti,da850-pll0", shall be "clksrc", "extclksrc"
>>> - for "ti,da850-pll1", shall be "clksrc"
>>> - #clock-cells: shall be set to <2>.
>>>
>>> Consumers:
>>>
>>> The clock cell values for consumers work as follows...
>>>
>>> The first index is one of the constants defined in ti-davinci-pll.h
>>>
>>> The second index is 0 unless the first index is TI_DAVINCI_SYSCLK. In
>>> the
>>> case
>>> of TI_DAVINCI_SYSCLK the second index the SYSCLK domain ID (n in
>>> SYSCLKn).
>>>
>>> For compatible = "ti,da850-pll0":
>>> - <&pll0 TI_DAVINCI_PLLOUT 0> is the PLLOUT clock
>>> - <&pll0 TI_DAVINCI_SYSCLK n> is one of the SYSCLKn clock
>>> domains
>>> where n is 1 to 7
>>> - <&pll0 TI_DAVINCI_AUXCLK 0> is the AUXCLK clock domain
>>> - <&pll0 TI_DAVINCI_OBSCLK 0> is the OBSCLK clock domain
>>> - all other index combinations are invalid
>>>
>>> For compatible = "ti,da850-pll1":
>>> - <&pll0 TI_DAVINCI_SYSCLK n> is one of the SYSCLKn clock
>>> domains
>>> where n is 1 to 3
>>> - <&pll0 TI_DAVINCI_OBSCLK 0> is the OBSCLK clock domain
>>> - all other index combinations are invalid
>>
>> You don't really need 2 cells here. I guess if you want to keep the
>> child nodes, that is fine.
>
> OK, I can see how it could work with one cell.
>
> Since this is already implemented and working, I'm inclined to leave it
> as-is if it is "good enough". But, I am fine going either way if there
> are other opinions on the matter.
FWIW, I think the current binding is fine too. The clocks are of
different kind (fixed clock, fixed divider, flexible divider etc). So,
its slightly better to have child nodes, I guess.
Thanks,
Sekhar
next prev parent reply other threads:[~2018-01-31 4:58 UTC|newest]
Thread overview: 131+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-01-20 17:13 [PATCH v6 00/41] ARM: davinci: convert to common clock framework David Lechner
2018-01-20 17:13 ` [PATCH v6 03/41] clk: davinci: Add platform information for TI DA830 PLL David Lechner
2018-02-01 8:10 ` Sekhar Nori
2018-01-20 17:13 ` [PATCH v6 04/41] clk: davinci: Add platform information for TI DA850 PLL David Lechner
2018-02-01 8:58 ` Sekhar Nori
[not found] ` <834cb7ce-9406-a806-3ec1-a59766bd8a9d-l0cyMroinI0@public.gmane.org>
2018-02-01 19:04 ` David Lechner
[not found] ` <6f0146e4-72bc-7bc2-2135-44950949cd77-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>
2018-02-02 8:23 ` Sekhar Nori
2018-02-01 19:22 ` David Lechner
[not found] ` <7d7e0522-30d5-6232-853e-7ab32fadfe48-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>
2018-02-02 8:37 ` Sekhar Nori
[not found] ` <ceec9849-e3fb-0e9b-4000-aab3ca2f5a43-l0cyMroinI0@public.gmane.org>
2018-02-02 17:45 ` David Lechner
2018-01-20 17:13 ` [PATCH v6 05/41] clk: davinci: Add platform information for TI DM355 PLL David Lechner
[not found] ` <1516468460-4908-6-git-send-email-david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>
2018-02-01 9:17 ` Sekhar Nori
2018-01-20 17:13 ` [PATCH v6 06/41] clk: davinci: Add platform information for TI DM365 PLL David Lechner
2018-02-01 9:28 ` Sekhar Nori
2018-01-20 17:13 ` [PATCH v6 07/41] clk: davinci: Add platform information for TI DM644x PLL David Lechner
2018-01-20 17:13 ` [PATCH v6 08/41] clk: davinci: Add platform information for TI DM646x PLL David Lechner
2018-01-20 17:13 ` [PATCH v6 09/41] dt-bindings: clock: New bindings for TI Davinci PSC David Lechner
2018-01-22 15:05 ` Rob Herring
2018-01-20 17:13 ` [PATCH v6 10/41] clk: davinci: New driver for davinci PSC clocks David Lechner
2018-02-01 9:55 ` Sekhar Nori
2018-01-20 17:13 ` [PATCH v6 11/41] clk: davinci: Add platform information for TI DA830 PSC David Lechner
2018-02-01 11:34 ` Sekhar Nori
2018-01-20 17:13 ` [PATCH v6 12/41] clk: davinci: Add platform information for TI DA850 PSC David Lechner
2018-02-01 11:42 ` Sekhar Nori
2018-01-20 17:13 ` [PATCH v6 13/41] clk: davinci: Add platform information for TI DM355 PSC David Lechner
2018-02-01 11:50 ` Sekhar Nori
2018-01-20 17:13 ` [PATCH v6 14/41] clk: davinci: Add platform information for TI DM365 PSC David Lechner
2018-02-01 11:55 ` Sekhar Nori
2018-01-20 17:13 ` [PATCH v6 15/41] clk: davinci: Add platform information for TI DM644x PSC David Lechner
2018-02-01 12:13 ` Sekhar Nori
2018-01-20 17:13 ` [PATCH v6 16/41] clk: davinci: Add platform information for TI DM646x PSC David Lechner
2018-02-01 12:17 ` Sekhar Nori
2018-01-20 17:13 ` [PATCH v6 17/41] dt-bindings: clock: Add bindings for DA8XX CFGCHIP clocks David Lechner
2018-01-29 19:59 ` Rob Herring
2018-02-02 6:20 ` Sekhar Nori
2018-02-02 17:50 ` David Lechner
2018-02-05 9:42 ` Sekhar Nori
2018-01-20 17:13 ` [PATCH v6 18/41] clk: davinci: New driver for TI " David Lechner
[not found] ` <1516468460-4908-19-git-send-email-david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>
2018-02-02 13:19 ` Sekhar Nori
[not found] ` <c2138795-edcd-bb7d-094d-47dc049c33df-l0cyMroinI0@public.gmane.org>
2018-02-02 13:53 ` Sekhar Nori
2018-02-02 17:56 ` David Lechner
2018-01-20 17:13 ` [PATCH v6 20/41] ARM: da830: add new clock init using common clock framework David Lechner
[not found] ` <1516468460-4908-21-git-send-email-david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>
2018-01-22 17:15 ` David Lechner
2018-02-02 14:12 ` Sekhar Nori
2018-02-02 18:03 ` David Lechner
2018-02-05 11:06 ` Sekhar Nori
2018-01-20 17:14 ` [PATCH v6 23/41] ARM: dm365: " David Lechner
2018-02-02 14:37 ` Sekhar Nori
2018-01-20 17:14 ` [PATCH v6 25/41] ARM: dm646x: " David Lechner
2018-02-02 14:55 ` Sekhar Nori
2018-01-20 17:14 ` [PATCH v6 26/41] ARM: da8xx: add new USB PHY " David Lechner
[not found] ` <1516468460-4908-27-git-send-email-david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>
2018-01-22 17:17 ` David Lechner
2018-01-20 17:14 ` [PATCH v6 27/41] ARM: da8xx: add new sata_refclk " David Lechner
2018-02-02 14:59 ` Sekhar Nori
2018-01-20 17:14 ` [PATCH v6 29/41] ARM: davinci_all_defconfig: remove CONFIG_DAVINCI_RESET_CLOCKS David Lechner
2018-02-02 15:04 ` Sekhar Nori
2018-01-20 17:14 ` [PATCH v6 30/41] ARM: davinci: switch to common clock framework David Lechner
2018-02-07 13:20 ` Sekhar Nori
2018-01-20 17:14 ` [PATCH v6 31/41] ARM: da830: Remove legacy clock init David Lechner
2018-02-07 13:28 ` Sekhar Nori
2018-01-20 17:14 ` [PATCH v6 33/41] ARM: dm355: " David Lechner
2018-02-07 13:42 ` Sekhar Nori
2018-01-20 17:14 ` [PATCH v6 34/41] ARM: dm365: " David Lechner
2018-02-07 13:44 ` Sekhar Nori
2018-01-20 17:14 ` [PATCH v6 35/41] ARM: dm644x: " David Lechner
[not found] ` <1516468460-4908-36-git-send-email-david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>
2018-02-07 13:46 ` Sekhar Nori
[not found] ` <1516468460-4908-1-git-send-email-david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>
2018-01-20 17:13 ` [PATCH v6 01/41] dt-bindings: clock: Add new bindings for TI Davinci PLL clocks David Lechner
2018-01-29 19:53 ` Rob Herring
2018-01-29 21:14 ` David Lechner
2018-01-30 14:50 ` Rob Herring
2018-01-30 18:46 ` David Lechner
2018-01-31 4:58 ` Sekhar Nori [this message]
2018-01-20 17:13 ` [PATCH v6 02/41] clk: davinci: New driver for davinci " David Lechner
[not found] ` <1516468460-4908-3-git-send-email-david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>
2018-02-01 8:01 ` Sekhar Nori
2018-02-01 12:22 ` Sekhar Nori
2018-02-01 18:57 ` David Lechner
[not found] ` <3ed91881-8753-a541-31aa-c835329141b3-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>
2018-02-02 8:12 ` Sekhar Nori
2018-01-20 17:13 ` [PATCH v6 19/41] clk: davinci: New driver for TI DA8XX USB PHY clocks David Lechner
2018-02-02 13:59 ` Sekhar Nori
2018-01-20 17:14 ` [PATCH v6 21/41] ARM: da850: add new clock init using common clock framework David Lechner
2018-02-02 14:20 ` Sekhar Nori
2018-02-02 18:05 ` David Lechner
2018-01-20 17:14 ` [PATCH v6 22/41] ARM: dm355: " David Lechner
2018-02-02 14:36 ` Sekhar Nori
2018-01-20 17:14 ` [PATCH v6 24/41] ARM: dm644x: " David Lechner
2018-02-02 14:39 ` Sekhar Nori
2018-02-02 18:06 ` David Lechner
[not found] ` <d8325090-e369-5a1a-b34a-8174123b01ff-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>
2018-02-05 6:01 ` Sekhar Nori
2018-01-20 17:14 ` [PATCH v6 28/41] ARM: davinci: remove CONFIG_DAVINCI_RESET_CLOCKS David Lechner
2018-02-02 15:03 ` Sekhar Nori
2018-01-20 17:14 ` [PATCH v6 32/41] ARM: da850: Remove legacy clock init David Lechner
2018-02-07 13:35 ` Sekhar Nori
2018-01-20 17:14 ` [PATCH v6 36/41] ARM: dm646x: " David Lechner
2018-02-07 15:06 ` Sekhar Nori
2018-01-20 17:14 ` [PATCH v6 38/41] ARM: davinci: remove legacy clocks David Lechner
[not found] ` <1516468460-4908-39-git-send-email-david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>
2018-02-07 15:24 ` Sekhar Nori
2018-01-20 17:14 ` [PATCH v6 39/41] ARM: davinci: add device tree support to timer David Lechner
2018-01-20 17:14 ` [PATCH v6 37/41] ARM: da8xx: Remove legacy clock init David Lechner
2018-02-07 15:16 ` Sekhar Nori
2018-01-20 17:14 ` [PATCH v6 40/41] ARM: da8xx-dt: switch to device tree clocks David Lechner
2018-01-24 3:26 ` David Lechner
[not found] ` <335d92c8-1af2-da64-e366-95d7513ad69d-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>
2018-02-05 14:04 ` Bartosz Golaszewski
2018-02-05 15:33 ` Bartosz Golaszewski
2018-01-20 17:14 ` [PATCH v6 41/41] ARM: dts: da850: Add clocks David Lechner
2018-01-22 17:14 ` David Lechner
[not found] ` <1516468460-4908-42-git-send-email-david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>
2018-01-24 4:08 ` David Lechner
2018-02-09 12:46 ` Sekhar Nori
2018-01-21 21:19 ` [PATCH v6 00/41] ARM: davinci: convert to common clock framework Adam Ford
2018-01-22 11:14 ` Bartosz Golaszewski
[not found] ` <CAMRc=MexJTMiD=URw1bv-qfGaTntZRXgfsSb_beYGkvO+LrpgQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-01-22 17:30 ` David Lechner
[not found] ` <d819e18f-7d74-d8c4-8056-bfc545f4d4e9-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>
2018-01-23 14:54 ` Bartosz Golaszewski
2018-01-23 16:03 ` David Lechner
2018-01-23 16:06 ` David Lechner
2018-01-23 17:03 ` Adam Ford
2018-01-23 18:10 ` Bartosz Golaszewski
2018-01-23 18:26 ` David Lechner
2018-01-23 18:34 ` Bartosz Golaszewski
2018-01-23 19:24 ` David Lechner
2018-01-23 19:53 ` Bartosz Golaszewski
[not found] ` <CAMRc=Me8HefZJYTkprqL2vpq_nLym4r6p-du7hoKwVk8kSCR8w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-01-23 20:01 ` David Lechner
2018-01-23 20:05 ` David Lechner
[not found] ` <f28601f6-d6f1-9a55-f3cf-62543412bd36-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>
2018-01-23 20:23 ` David Lechner
[not found] ` <fc54ab26-72be-1377-2ccb-72d34360c5eb-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>
2018-01-24 8:03 ` Bartosz Golaszewski
2018-01-25 12:53 ` Sekhar Nori
2018-01-25 13:34 ` Bartosz Golaszewski
2018-01-25 16:18 ` David Lechner
2018-01-25 17:05 ` Sekhar Nori
2018-01-23 17:04 ` Bartosz Golaszewski
2018-01-22 13:29 ` Bartosz Golaszewski
2018-01-22 17:11 ` David Lechner
2018-01-23 14:56 ` Bartosz Golaszewski
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=a31b79a0-cf09-8c59-5b14-33daf6c1dccb@ti.com \
--to=nsekhar@ti.com \
--cc=aford173@gmail.com \
--cc=bgolaszewski@baylibre.com \
--cc=david@lechnology.com \
--cc=devicetree@vger.kernel.org \
--cc=khilman@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-clk@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mturquette@baylibre.com \
--cc=robh@kernel.org \
--cc=sboyd@codeaurora.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).