Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752473AbeAJWY2 (ORCPT + 1 other); Wed, 10 Jan 2018 17:24:28 -0500 Received: from mail-vk0-f68.google.com ([209.85.213.68]:34818 "EHLO mail-vk0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751957AbeAJWYP (ORCPT ); Wed, 10 Jan 2018 17:24:15 -0500 X-Google-Smtp-Source: ACJfBov8RrKqtBJl0Ia/IKO48befTmhDLmeN9dtTrJV0wcK9DppQ6okJaayRYCH0pRro0XZjWteKds6VeNT3GbIK/CY= MIME-Version: 1.0 In-Reply-To: <5aacc350-6236-2e4f-35bb-a681fc9d47e7@ti.com> References: <1515377863-20358-1-git-send-email-david@lechnology.com> <1515377863-20358-2-git-send-email-david@lechnology.com> <22409e49-5c14-4068-b137-7535afaf90d7@lechnology.com> <0f90b5f7-f21e-5f81-1154-9a815bbb786d@ti.com> <5aacc350-6236-2e4f-35bb-a681fc9d47e7@ti.com> From: Adam Ford Date: Wed, 10 Jan 2018 16:24:13 -0600 Message-ID: Subject: Re: [PATCH v5 01/44] dt-bindings: clock: Add new bindings for TI Davinci PLL clocks To: Sekhar Nori Cc: David Lechner , linux-clk@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Michael Turquette , Stephen Boyd , Rob Herring , Mark Rutland , Kevin Hilman , linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: On Wed, Jan 10, 2018 at 12:52 PM, Sekhar Nori wrote: > On Wednesday 10 January 2018 08:31 AM, David Lechner wrote: >> On 01/09/2018 06:35 AM, Sekhar Nori wrote: >>> On Monday 08 January 2018 09:59 PM, David Lechner wrote: >>>> On 01/08/2018 08:00 AM, Sekhar Nori wrote: >>>>> On Monday 08 January 2018 07:47 AM, David Lechner wrote: > >>>>>> 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..99bf5da >>>>>> --- /dev/null >>>>>> +++ b/Documentation/devicetree/bindings/clock/ti/davinci/pll.txt >>>>>> @@ -0,0 +1,47 @@ >>>>>> +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 >>>>> >>>>> These PLLs are same IP so they should use the same compatible. You can >>>>> initialize both PLLs for DA850 based on the same compatible. >>>>> >>>> >>>> But they are not exactly the same. For example, PLL0 has 7 PLLDIV clocks >>>> while >>>> PLL1 only has 3. PLL0 has PREDIV while PLL1 does not. PLL0 has certain >>>> SYSCLKs >>>> that are fixed-ratio but PLL1 does not have any of these. There are even >>>> more >>>> differences, but these are the ones we are actually using. >>> >>> We need each element of the PLLC to be modeled individually as a clock >>> node. >> >> I gave this a good think while I have been working on this series >> and I came to the conclusion that we really don't need to do this. >> These components are all internal to the PLL IP block, so the >> compatible string is enough to tell us what we have. They only >> thing we need really in the device tree bindings are the connections >> that are external to the IP block. >> >> >>> That is, PLL should only model the multiplier, the dividers >>> including post and prediv should be modeled as divider clocks (hopefully >>> being able to use the clk-divider.c library). The sysclks can be >>> fixed-factor-clock type clocks. >>> >>> Without this flexible mechanism, we cannot (at least later) model things >>> like DIV4.5 clock which is the only clock which derives from the output >>> of PLL multiplier before the post divider is applied. >>> >>> Since with DT there are are no retakes, we need to get this right the >>> first time and modifying later will not be an option. >>> >> >> So, the full device tree binding would look something like this: >> >> + >> + pll0: clock-controller@11000 { >> + compatible = "ti,da850-pll0"; >> + reg = <0x11000 0x1000>; >> + clocks = <&ref_clk>, <&pll1_sysclk 3>, <&pll1_obsclk>; >> + clock-names = "oscin", pll1_sysclk3", "pll1_osbclk"; >> + oscin-square-wave; >> + >> + pll0_sysclk: sysclk { >> + #clock-cells = <1>; >> + }; >> + >> + pll0_auxclk: auxclk { >> + #clock-cells = <0>; >> + }; >> + >> + pll0_div45: div4.5 { >> + #clock-cells = <0>; >> + }; >> + >> + pll0_obsclk: obsclk { >> + #clock-cells = <0>; >> + assigned-clocks = <&pll0_sysclk 1>; >> + assigned-clock-names = "ocsrc"; >> + }; >> + }; > > Well, I guess this will work as well. And I am probably biased towards > the style I mentioned because AM335x and other TI OMAP processors > follow that. > > To make it easy to review that we have all bases covered, can you model > the all PLLC0 and PLLC1 (input and output) clocks for the next version? > >> >> There are three clocks coming into the IP block and there are 11 clocks >> going out (sysclk is 7 clocks). And you can specify the board-specific >> configuration, like having the "oscin-square-wave" flag when a square wave >> is used instead of a crystal oscillator and you can assign the multiplexer > > Ideally the OSCIN vs CLKIN selection should be another clock mux whose > output is one of the input clocks to PLL controller. But I can see the > difficulty in handling that as the mux itself is controlled by the PLL > controller. > >> input that will be used by obsclk. (And, this binding is totally compatible >> with the binding I have already proposed - although, I see now it would >> be better to go ahead and add the clocks-names property.) > > Also, please add the oscin-square-wave to the binding definition too. > > For the benefit of others reviewing and not familiar with the hardware, > the users guide for DA850 is here: > http://www.ti.com/lit/ug/spruh77c/spruh77c.pdf > I am available tomorrow to build and test patches against the da850-evm. I just need to know which version(s) to test. adam > and the PLL block diagram is on page 143 (Figure 8-1). > > Thanks, > Sekhar