From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Warren Subject: Re: [PATCH v4 6/6] clk: Add floor and ceiling constraints to clock rates Date: Thu, 31 Jul 2014 11:51:20 -0600 Message-ID: <53DA8218.7070300@wwwdotorg.org> 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> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; Format="flowed" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <53DA2CCB.2080300@collabora.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: linuxppc-dev-bounces+glppe-linuxppc-embedded-2=m.gmane.org@lists.ozlabs.org Sender: "Linuxppc-dev" To: Tomeu Vizoso , Mike Turquette 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, Vi List-Id: alsa-devel@alsa-project.org T24gMDcvMzEvMjAxNCAwNTo0NyBBTSwgVG9tZXUgVml6b3NvIHdyb3RlOgo+IE9uIDA3LzIyLzIw MTQgMDc6NTAgUE0sIFN0ZXBoZW4gV2FycmVuIHdyb3RlOgo+PiBPbiAwNy8xNy8yMDE0IDA4OjEz IEFNLCBUb21ldSBWaXpvc28gd3JvdGU6Cj4+PiBBZGRzIGEgd2F5IGZvciBjbG9jayBjb25zdW1l cnMgdG8gc2V0IG1heGltdW0gYW5kIG1pbmltdW0gcmF0ZXMuIFRoaXMKPj4+IGNhbiBiZQo+Pj4g dXNlZCBmb3IgdGhlcm1hbCBkcml2ZXJzIHRvIHNldCBjZWlsaW5nIHJhdGVzLCBvciBieSBtaXNj LiBkcml2ZXJzIHRvCj4+PiBzZXQKPj4+IGZsb29yIHJhdGVzIHRvIGFzc3VyZSBhIG1pbmltdW0g cGVyZm9ybWFuY2UgbGV2ZWwuCj4+Cj4+PiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9jbGsvY2xrLmMg Yi9kcml2ZXJzL2Nsay9jbGsuYwo+Pgo+Pj4gK3N0cnVjdCByYXRlX2NvbnN0cmFpbnQgewo+Pj4g KyAgICBzdHJ1Y3QgaGxpc3Rfbm9kZSAgICBub2RlOwo+Pj4gKyAgICBjb25zdCBjaGFyICAgICAg ICAqZGV2X2lkOwo+Pj4gKyAgICBjb25zdCBjaGFyICAgICAgICAqY29uX2lkOwo+Pj4gKyAgICBl bnVtIGNvbnN0cmFpbnRfdHlwZSAgICB0eXBlOwo+Pj4gKyAgICB1bnNpZ25lZCBsb25nICAgICAg ICByYXRlOwo+Pj4gK307Cj4+Cj4+IEkgd291bGQgcGVyc29uYWxseSBzdGlsbCBwcmVmZXIgZWl0 aGVyOgo+Pgo+PiBhKSBUaGUgc3RydWN0IHJhdGVfY29uc3RyYWludHMgYmUgc3RvcmVkIGluIHN0 cnVjdCBjbGsgcmF0aGVyIHRoYW4KPj4gc3RydWN0IGNsa19jb3JlLCBzbyB0aGV5J3JlIHN0b3Jl ZCBpbiB0aGUgY29udGFpbmVyIHRoYXQgInNldCIgdGhlCj4+IGNvbnN0cmFpbnRzLiBUaGlzIHdv dWxkIG1lYW4gaXRlcmF0aW5nIG92ZXIgYSBzdHJ1Y3QgY2xrX2NvcmUncwo+PiBhc3NvY2lhdGVk IHN0cnVjdCBjbGtzLCB0aGVuIGl0ZXJhdGluZyBvdmVyIHRoZSBzdHJ1Y3QgcmF0ZV9jb25zdHJh aW50cwo+PiB3aXRoaW4gZWFjaCBzdHJ1Y3QgY2xrLgo+Pgo+PiBvcjoKPj4KPj4gYikgc3RydWN0 IHJhdGVfY29uc3RyYWludCB0byBjb250YWluIGEgcG9pbnRlciB0byB0aGUgc3RydWN0IGNsayB0 aGF0Cj4+IGNyZWF0ZWQgdGhlIGNvbnN0cmFpbnQsIHJhdGhlciB0aGFuIGNvcHlpbmcgdGhlIGRl dl9pZC9jb25faWQgZnJvbSB0aGF0Cj4+IHN0cnVjdCBjbGsgaW50byB0aGUgc3RydWN0IHJhdGVf Y29uc3RyYWludC4KPj4KPj4gV2l0aCBlaXRoZXIgb2YgdGhvc2UgY2hhbmdlcywgdGhlIGNvbnN0 cmFpbnRzIGFyZSBkaXJlY3RseSBhc3NvY2lhdGVkCj4+IHdpdGggdGhlIGNsb2NrIGNsaWVudCBv YmplY3QgdGhhdCBjcmVhdGVkIHRoZSBjb25zdHJhaW50IG11Y2ggYmV0dGVyCj4+IHRoYW4gcmln aHQgbm93LCB3aGVyZSB0aGUgbWF0Y2hpbmcgaXMgb25seSBiZWNhdXNlIHRoZSBzdHJ1Y3QgY2xr IGFuZAo+PiBzdHJ1Y3QgcmF0ZV9jb25zdHJhaW50ICJoYXBwZW4gdG8iIGhhdmUgdGhlIHNhbWUg ZGV2X2lkL2Nvbl9pZCB2YWx1ZXMuCj4+Cj4+IFN0aWxsLCB0aGlzIGlzIGEgcHJldHR5IG1pbm9y IGlzc3VlLCBhbmQgb3ZlcmFsbCB0aGlzIHNlcmllcyBsb29rcwo+PiByZWFzb25hYmxlIHRvIG1l IGF0IGEgcXVpY2sgbG9vay4KPgo+IFllYWgsIEkgYWdyZWUgYW5kIHBlcnNvbmFsbHkgcHJlZmVy IGEpLCBidXQgZ2l2ZW4gdGhlIGxpdHRsZSBmZWVkYmFjawo+IHRoYXQgSSBoYXZlIGdvdHRlbiBz byBmYXIgb24gdGhlIHJlZmFjdG9yaW5nLCBJJ20gc3RhcnRpbmcgdG8gY29uc2lkZXIKPiBmb3Jn ZXR0aW5nIGFib3V0IHRoZSBwZXItdXNlciBjbGsgc3RydWN0IGFuZCBnbyBpbnN0ZWFkIHdpdGgg YQo+IHJlcXVlc3QtYmFzZWQgQVBJIHNpbWlsYXIgdG8gdGhhdCBvZiBwbV9xb3MsIGZvciBzZXR0 aW5nIGZsb29yIGFuZAo+IGNlaWxpbmcgZnJlcXVlbmNpZXMuCgpJIHdvdWxkIG9idmlvdXNseSBl bmNvdXJhZ2UgeW91IHRvIGNvbnRpbnVlIHB1c2hpbmcgZm9yIHRoaXMgZmVhdHVyZSwgCmFsdGhv dWdoIEkgdW5kZXJzdGFuZCBpdCBjYW4gYmUgZGlmZmljdWx0IHRvIGJlIG1vdGl2YXRlZCB3aGVu IHRoZSAKcGF0Y2hlcyBkb24ndCBnZXQgbXVjaCBmZWVkYmFjay4KX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX18KTGludXhwcGMtZGV2IG1haWxpbmcgbGlzdApM aW51eHBwYy1kZXZAbGlzdHMub3psYWJzLm9yZwpodHRwczovL2xpc3RzLm96bGFicy5vcmcvbGlz dGluZm8vbGludXhwcGMtZGV2 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.