From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from avon.wwwdotorg.org (avon.wwwdotorg.org [70.85.31.133]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 8BF801A0224 for ; Fri, 1 Aug 2014 03:52:53 +1000 (EST) Message-ID: <53DA8218.7070300@wwwdotorg.org> Date: Thu, 31 Jul 2014 11:51:20 -0600 From: Stephen Warren MIME-Version: 1.0 To: Tomeu Vizoso , Mike Turquette Subject: Re: [PATCH v4 6/6] clk: Add floor and ceiling constraints to clock rates References: <1405606399-26608-1-git-send-email-tomeu.vizoso@collabora.com> <1405606399-26608-7-git-send-email-tomeu.vizoso@collabora.com> <53CEA483.6090704@wwwdotorg.org> <53DA2CCB.2080300@collabora.com> In-Reply-To: <53DA2CCB.2080300@collabora.com> Content-Type: text/plain; charset=windows-1252; format=flowed Cc: Andrew Lunn , Ulf Hansson , Prashant Gaikwad , Tony Lindgren , Tomasz Figa , alsa-devel@alsa-project.org, Jaroslav Kysela , Thierry Reding , Paul Mackerras , Sylwester Nawrocki , Daniel Walker , linux-arch@vger.kernel.org, Boris Brezillon , linux-samsung-soc@vger.kernel.org, Kukjin Kim , Russell King , =?windows-1252?Q?Emilio_L=F3pez?= , Takashi Iwai , Michal Simek , Kyungmin Park , Kevin Hilman , linux-arm-kernel@lists.infradead.org, patches@opensource.wolfsonmicro.com, Viresh Kumar , David Brown , Anatolij Gustschin , Dinh Nguyen , Sebastian Hesselbarth , Jason Cooper , Arnd Bergmann , linux-arm-msm@vger.kernel.org, spear-devel@list.st.com, Barry Song , Mark Brown , linux-rpi-kernel@lists.infradead.org, Ben Dooks , linux-tegra@vger.kernel.org, linux-omap@vger.kernel.org, Sascha Hauer , Shawn Guo , Paul Walmsley , Peter De Schrijver , Liam Girdwood , linux-kernel@vger.kernel.org, rabin@rab.in, Bryan Huntsman , Santosh Shilimkar , =?windows-1252?Q?Beno=EEt_Cousson?= , Maxime Ripard , linux-media@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, Javier Martinez Canillas , Mauro Carvalho Chehab List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On 07/31/2014 05:47 AM, Tomeu Vizoso wrote: > On 07/22/2014 07:50 PM, Stephen Warren wrote: >> On 07/17/2014 08:13 AM, Tomeu Vizoso wrote: >>> Adds a way for clock consumers to set maximum and minimum rates. This >>> can be >>> used for thermal drivers to set ceiling rates, or by misc. drivers to >>> set >>> floor rates to assure a minimum performance level. >> >>> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c >> >>> +struct rate_constraint { >>> + struct hlist_node node; >>> + const char *dev_id; >>> + const char *con_id; >>> + enum constraint_type type; >>> + unsigned long rate; >>> +}; >> >> I would personally still prefer either: >> >> a) The struct rate_constraints be stored in struct clk rather than >> struct clk_core, so they're stored in the container that "set" the >> constraints. This would mean iterating over a struct clk_core's >> associated struct clks, then iterating over the struct rate_constraints >> within each struct clk. >> >> or: >> >> b) struct rate_constraint to contain a pointer to the struct clk that >> created the constraint, rather than copying the dev_id/con_id from that >> struct clk into the struct rate_constraint. >> >> With either of those changes, the constraints are directly associated >> with the clock client object that created the constraint much better >> than right now, where the matching is only because the struct clk and >> struct rate_constraint "happen to" have the same dev_id/con_id values. >> >> Still, this is a pretty minor issue, and overall this series looks >> reasonable to me at a quick look. > > Yeah, I agree and personally prefer a), but given the little feedback > that I have gotten so far on the refactoring, I'm starting to consider > forgetting about the per-user clk struct and go instead with a > request-based API similar to that of pm_qos, for setting floor and > ceiling frequencies. I would obviously encourage you to continue pushing for this feature, although I understand it can be difficult to be motivated when the patches don't get much feedback.