Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932925Ab3HGRyr (ORCPT ); Wed, 7 Aug 2013 13:54:47 -0400 Received: from mail-oa0-f46.google.com ([209.85.219.46]:50872 "EHLO mail-oa0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932663Ab3HGRyq (ORCPT ); Wed, 7 Aug 2013 13:54:46 -0400 MIME-Version: 1.0 In-Reply-To: <52028859.20008@wwwdotorg.org> References: <8d192a13cb7e088943da40689d62bc6353bd8604.1375886595.git.viresh.kumar@linaro.org> <52028620.6000608@wwwdotorg.org> <52028859.20008@wwwdotorg.org> Date: Wed, 7 Aug 2013 23:24:45 +0530 Message-ID: Subject: Re: [PATCH 1/6] clk: Tegra: Add CPU0 clock driver From: Viresh Kumar To: Stephen Warren 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 Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2165 Lines: 47 On 7 August 2013 23:18, Stephen Warren wrote: > On 08/07/2013 11:45 AM, Viresh Kumar wrote: >> On 7 August 2013 23:08, Stephen Warren wrote: >>> On 08/07/2013 08:46 AM, Viresh Kumar wrote: >>>> This patch adds CPU0's clk driver for Tegra. It will be used by the generic >>>> cpufreq-cpu0 driver to get/set cpu clk. >>>> >>>> Most of the platform specific bits are picked from tegra-cpufreq.c. >>> >>> Hmmm. I'm not sure if it makes sense to represent this as a clock >>> object; isn't this more of a virtual construct that manages the rate of >>> the clock, rather than an actual clock? The actual clock already exists >>> as "cpu". >> >> I see it as this: There is a clock in system for cpu, call it "cpu". Now we >> must be able to provide get/set routines for it. A set should set the >> frequency to whatever is asked for and should really worry about how >> that is being set. This part is internal to "cpu" clk. > > Sure. > >> This is what cpufreq-cpu0 driver should expect and does. Current "cpu" >> clock implemented doesn't provide this facility ? And so this wrapper >> made sense to me. > > But the additional management logic on top of the raw clock is exactly > what the cpufreq driver is for. This patch series is basically moving > the cpufreq driver code inside the clock code instead. Above "sure" didn't go very well with what you wrote here :) The additional management that we are required to do isn't cpufreq driver specific but cpu or platform specific. cpufreq shouldn't care about how CPU's clock is set to a particular frequency, its headache of CPU's clk driver instead. cpu is yet another device and so clk_set_rate() must be enough to set its frequency.... There might be other frameworks that need to set frequency of this device later on and surely we don't want to replicate such piece of code to every user.. Does it make sense to you? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/