From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Warren Subject: Re: [PATCH 2/6] ARM: Tegra: Add CPU's OPPs for using cpufreq-cpu0 driver Date: Wed, 07 Aug 2013 12:55:13 -0600 Message-ID: <52029811.9080704@wwwdotorg.org> References: <52028719.8070908@wwwdotorg.org> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org List-ID: Content-Type: text/plain; charset="us-ascii" To: Viresh Kumar Cc: rjw@sisk.pl, swarren@nvidia.com, linaro-kernel@lists.linaro.org, patches@linaro.org, cpufreq@vger.kernel.org, linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org, mturquette@linaro.org, linux-arm-kernel@lists.infradead.org, "devicetree@vger.kernel.org" , Ian Campbell , Mark Rutland , Pawel Moll , Rob Herring On 08/07/2013 12:06 PM, Viresh Kumar wrote: > On 7 August 2013 23:12, Stephen Warren wrote: >> On 08/07/2013 08:46 AM, Viresh Kumar wrote: >>> cpufreq-cpu0 driver needs OPPs to be present in DT which can be probed by it to >>> get frequency table. This patch adds OPPs and clock-latency to tegra cpu0 node >>> for multiple SoCs. >>> >>> Voltage levels aren't used until now for tegra and so a flat value which would >>> eventually be ignored is used to represent voltage. >> >> This patch is problematic w.r.t. DT being an ABI. > > :( > >> We can certainly add new optional properties to a DT binding that enable >> new features. However, a new version of a binding can't require new >> properties to exist that didn't before, since that means that old DTs >> won't work with new kernels that require the new properties. > > To be honest I didn't get it completely. You meant operating-points > wasn't present before? Its here: > > Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt > Documentation/devicetree/bindings/power/opp.txt > > Or you meant, Tegra never required voltage levels and we are getting > them in here. The current Tegra *.dts files do not contain this property. The current Tegra *.dts files must continue to work without modification in future kernels. >> As such, I believe we do need some Tegra-specific piece of code that >> defines these OPP tables in the kernel, so that the operating-points >> property is not needed. > > Generic cpufreq driver depends on OPP library and so somebody has > to provide them. Now you can do it by calling opp_add() for each OPP > you have or otherwise. Sure. That's what the Tegra-specific cpufreq driver should do. It should be the top-level cpufreq driver. If parts of the code can be implemented by library functions or a core parameterizable driver, then presumably the Tegra driver would simply exist to provide those parameters and/or callback functions to the generic driver. > Btw, you must have some specific voltage level for each freq, we can > get them here.. Yes, I'm sure we do, but I have no idea what they are:-( It may even be board-specific or SoC-SKU-specific. I think we should defer this aspect for now.