From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752601AbbBMKTL (ORCPT ); Fri, 13 Feb 2015 05:19:11 -0500 Received: from mail.kapsi.fi ([217.30.184.167]:46924 "EHLO mail.kapsi.fi" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752115AbbBMKTI (ORCPT ); Fri, 13 Feb 2015 05:19:08 -0500 Message-ID: <54DDCF8B.1050903@kapsi.fi> Date: Fri, 13 Feb 2015 12:18:51 +0200 From: Mikko Perttunen User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.3.0 MIME-Version: 1.0 To: Thierry Reding CC: swarren@wwwdotorg.org, gnurou@gmail.com, pdeschrijver@nvidia.com, rjw@rjwysocki.net, viresh.kumar@linaro.org, mturquette@linaro.org, pwalmsley@nvidia.com, vinceh@nvidia.com, pgaikwad@nvidia.com, linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org, linux-tegra@vger.kernel.org, linux-arm-kernel@lists.infradead.org, tuomas.tynkkynen@iki.fi, Tuomas Tynkkynen Subject: Re: [PATCH v7 01/16] clk: tegra: Add binding for the Tegra124 DFLL clocksource References: <1420723339-30735-1-git-send-email-mikko.perttunen@kapsi.fi> <1420723339-30735-2-git-send-email-mikko.perttunen@kapsi.fi> <20150212224242.GA23500@mithrandir> In-Reply-To: <20150212224242.GA23500@mithrandir> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-SA-Exim-Connect-IP: 2001:708:30:12d0:beee:7bff:fe5b:f272 X-SA-Exim-Mail-From: mikko.perttunen@kapsi.fi X-SA-Exim-Scanned: No (on mail.kapsi.fi); SAEximRunCond expanded to false Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 02/13/2015 12:42 AM, Thierry Reding wrote: > On Thu, Jan 08, 2015 at 03:22:04PM +0200, Mikko Perttunen wrote: >> From: Tuomas Tynkkynen >> >> The DFLL is the main clocksource for the fast CPU cluster on Tegra124 >> and also provides automatic CPU rail voltage scaling as well. The DFLL >> is a separate IP block from the usual Tegra124 clock-and-reset >> controller, so it gets its own node in the device tree. >> >> Signed-off-by: Tuomas Tynkkynen >> Signed-off-by: Mikko Perttunen >> --- >> .../bindings/clock/nvidia,tegra124-dfll.txt | 69 ++++++++++++++++++++++ >> 1 file changed, 69 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt >> >> diff --git a/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt b/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt >> new file mode 100644 >> index 0000000..54c62ac >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt >> @@ -0,0 +1,69 @@ >> +NVIDIA Tegra124 DFLL FCPU clocksource >> + >> +This binding uses the common clock binding: >> +Documentation/devicetree/bindings/clock/clock-bindings.txt >> + >> +The DFLL IP block on Tegra is a root clocksource designed for clocking >> +the fast CPU cluster. It consists of a free-running voltage controlled >> +oscillator connected to the CPU voltage rail (VDD_CPU), and a closed loop >> +control module that will automatically adjust the VDD_CPU voltage by >> +communicating with an off-chip PMIC either via an I2C bus or via PWM signals. > > How would PWM communication work? The documentation below doesn't > describe it, so perhaps either leave that part out until the binding is > updated to support it, or maybe make an explicit note that this mode of > operation isn't supported yet? Yes, I'll edit this part. > >> +Required properties: >> +- compatible : should be "nvidia,tegra124-dfll-fcpu" >> +- reg : Defines the following set of registers, in the order listed: >> + - registers for the DFLL control logic. >> + - registers for the I2C output logic. >> + - registers for the integrated I2C master controller. >> + - look-up table RAM for voltage register values. > > Why do these all need to be separate sets? According to the TRM this is > a single IP block with a single register region, why the need to split > them apart? I will have to check with Tuomas if he had a reason to do it this way. > >> +- interrupts: Should contain the DFLL block interrupt. >> +- clocks: Must contain an entry for each entry in clock-names. >> + See ../clocks/clock-bindings.txt for details. >> +- clock-names: Must include the following entries: >> + - soc: Clock source for the DFLL control logic. >> + - ref: The closed loop reference clock >> + - i2c: Clock source for the integrated I2C master. >> +- #clock-cells: Must be 0. >> +- clock-output-names: Name of the clock output. >> +- vdd-cpu-supply: Regulator for the CPU voltage rail that the DFLL >> + hardware will start controlling. > > How does the hardware control it? How do we go from regulator phandle to > something that the hardware will know how to access? Looks like this property doesn't exist in the current device tree, along with some other properties. The compatible-string noted above is wrong too. I'll fix these for the next version. > >> +Required properties for the control loop parameters: >> +- nvidia,sample-rate: Sample rate of the DFLL control loop. >> +- nvidia,droop-ctrl: See the register CL_DVFS_DROOP_CTRL in the TRM. >> +- nvidia,force-mode: See the field DFLL_PARAMS_FORCE_MODE in the TRM. >> +- nvidia,cf: Numeric value, see the field DFLL_PARAMS_CF_PARAM in the TRM. >> +- nvidia,ci: Numeric value, see the field DFLL_PARAMS_CI_PARAM in the TRM. >> +- nvidia,cg: Numeric value, see the field DFLL_PARAMS_CG_PARAM in the TRM. >> +- nvidia,cg-scale: Boolean value, see the field DFLL_PARAMS_CG_SCALE in the TRM. >> + >> +Required properties for I2C mode: >> +- nvidia,i2c-fs-rate: I2C transfer rate, if using FS mode. > > What's FS mode? I would imagine it stands for full speed. I'll expand the abbreviation. > >> + >> +Example: >> + >> +dfll@0,70110000 { > > Perhaps this should be "clock@0,70110000"? Probably. > >> + compatible = "nvidia,tegra124-dfll"; >> + reg = <0 0x70110000 0 0x100>, /* DFLL control */ >> + <0 0x70110000 0 0x100>, /* I2C output control */ >> + <0 0x70110100 0 0x100>, /* Integrated I2C controller */ >> + <0 0x70110200 0 0x100>; /* Look-up table RAM */ >> + interrupts = ; >> + clocks = <&tegra_car TEGRA124_CLK_DFLL_SOC>, >> + <&tegra_car TEGRA124_CLK_DFLL_REF>, >> + <&tegra_car TEGRA124_CLK_I2C5>; > > If this controls I2C5 now, should it be documented that we can't use > this controller in software while it is in use by DFLL? I'll add a note about this. > > Thierry > Cheers, Mikko.