From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [93.93.135.160]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id BBC961A0226 for ; Thu, 31 Jul 2014 21:58:05 +1000 (EST) Message-ID: <53DA2CCB.2080300@collabora.com> Date: Thu, 31 Jul 2014 13:47:23 +0200 From: Tomeu Vizoso MIME-Version: 1.0 To: Stephen Warren , 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> In-Reply-To: <53CEA483.6090704@wwwdotorg.org> Content-Type: text/plain; charset=ISO-8859-1; 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 , =?ISO-8859-1?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 , =?ISO-8859-1?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/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. Regards, Tomeu