From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mike Turquette Subject: Re: [PATCH v4 6/6] clk: Add floor and ceiling constraints to clock rates Date: Thu, 31 Jul 2014 11:54:44 -0700 Message-ID: <20140731185444.4463.35237@quantum> 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> <53DA8218.7070300@wwwdotorg.org> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <53DA8218.7070300@wwwdotorg.org> 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: Stephen Warren , Tomeu Vizoso Cc: Ulf Hansson , Prashant Gaikwad , "Jason Cooper , Andrew Lunn , Sebastian Hesselbarth , Russell King , Shawn Guo , Sascha Hauer , David Brown , Daniel Walker , Bryan Huntsman , Paul Walmsley , Tony Lindgren , =?utf-8?q?=22_Beno=C3=AEt_Cousson?= , Kevin Hilman" , Tomasz Figa , alsa-devel@alsa-project.org, Jaroslav Kysela , Thierry Reding , Paul Mackerras , Sylwester Nawrocki , linux-arch@vger.kernel.org List-Id: alsa-devel@alsa-project.org UXVvdGluZyBTdGVwaGVuIFdhcnJlbiAoMjAxNC0wNy0zMSAxMDo1MToyMCkKPiBPbiAwNy8zMS8y MDE0IDA1OjQ3IEFNLCBUb21ldSBWaXpvc28gd3JvdGU6Cj4gPiBPbiAwNy8yMi8yMDE0IDA3OjUw IFBNLCBTdGVwaGVuIFdhcnJlbiB3cm90ZToKPiA+PiBPbiAwNy8xNy8yMDE0IDA4OjEzIEFNLCBU b21ldSBWaXpvc28gd3JvdGU6Cj4gPj4+IEFkZHMgYSB3YXkgZm9yIGNsb2NrIGNvbnN1bWVycyB0 byBzZXQgbWF4aW11bSBhbmQgbWluaW11bSByYXRlcy4gVGhpcwo+ID4+PiBjYW4gYmUKPiA+Pj4g dXNlZCBmb3IgdGhlcm1hbCBkcml2ZXJzIHRvIHNldCBjZWlsaW5nIHJhdGVzLCBvciBieSBtaXNj LiBkcml2ZXJzIHRvCj4gPj4+IHNldAo+ID4+PiBmbG9vciByYXRlcyB0byBhc3N1cmUgYSBtaW5p bXVtIHBlcmZvcm1hbmNlIGxldmVsLgo+ID4+Cj4gPj4+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2Ns ay9jbGsuYyBiL2RyaXZlcnMvY2xrL2Nsay5jCj4gPj4KPiA+Pj4gK3N0cnVjdCByYXRlX2NvbnN0 cmFpbnQgewo+ID4+PiArICAgIHN0cnVjdCBobGlzdF9ub2RlICAgIG5vZGU7Cj4gPj4+ICsgICAg Y29uc3QgY2hhciAgICAgICAgKmRldl9pZDsKPiA+Pj4gKyAgICBjb25zdCBjaGFyICAgICAgICAq Y29uX2lkOwo+ID4+PiArICAgIGVudW0gY29uc3RyYWludF90eXBlICAgIHR5cGU7Cj4gPj4+ICsg ICAgdW5zaWduZWQgbG9uZyAgICAgICAgcmF0ZTsKPiA+Pj4gK307Cj4gPj4KPiA+PiBJIHdvdWxk IHBlcnNvbmFsbHkgc3RpbGwgcHJlZmVyIGVpdGhlcjoKPiA+Pgo+ID4+IGEpIFRoZSBzdHJ1Y3Qg cmF0ZV9jb25zdHJhaW50cyBiZSBzdG9yZWQgaW4gc3RydWN0IGNsayByYXRoZXIgdGhhbgo+ID4+ IHN0cnVjdCBjbGtfY29yZSwgc28gdGhleSdyZSBzdG9yZWQgaW4gdGhlIGNvbnRhaW5lciB0aGF0 ICJzZXQiIHRoZQo+ID4+IGNvbnN0cmFpbnRzLiBUaGlzIHdvdWxkIG1lYW4gaXRlcmF0aW5nIG92 ZXIgYSBzdHJ1Y3QgY2xrX2NvcmUncwo+ID4+IGFzc29jaWF0ZWQgc3RydWN0IGNsa3MsIHRoZW4g aXRlcmF0aW5nIG92ZXIgdGhlIHN0cnVjdCByYXRlX2NvbnN0cmFpbnRzCj4gPj4gd2l0aGluIGVh Y2ggc3RydWN0IGNsay4KPiA+Pgo+ID4+IG9yOgo+ID4+Cj4gPj4gYikgc3RydWN0IHJhdGVfY29u c3RyYWludCB0byBjb250YWluIGEgcG9pbnRlciB0byB0aGUgc3RydWN0IGNsayB0aGF0Cj4gPj4g Y3JlYXRlZCB0aGUgY29uc3RyYWludCwgcmF0aGVyIHRoYW4gY29weWluZyB0aGUgZGV2X2lkL2Nv bl9pZCBmcm9tIHRoYXQKPiA+PiBzdHJ1Y3QgY2xrIGludG8gdGhlIHN0cnVjdCByYXRlX2NvbnN0 cmFpbnQuCj4gPj4KPiA+PiBXaXRoIGVpdGhlciBvZiB0aG9zZSBjaGFuZ2VzLCB0aGUgY29uc3Ry YWludHMgYXJlIGRpcmVjdGx5IGFzc29jaWF0ZWQKPiA+PiB3aXRoIHRoZSBjbG9jayBjbGllbnQg b2JqZWN0IHRoYXQgY3JlYXRlZCB0aGUgY29uc3RyYWludCBtdWNoIGJldHRlcgo+ID4+IHRoYW4g cmlnaHQgbm93LCB3aGVyZSB0aGUgbWF0Y2hpbmcgaXMgb25seSBiZWNhdXNlIHRoZSBzdHJ1Y3Qg Y2xrIGFuZAo+ID4+IHN0cnVjdCByYXRlX2NvbnN0cmFpbnQgImhhcHBlbiB0byIgaGF2ZSB0aGUg c2FtZSBkZXZfaWQvY29uX2lkIHZhbHVlcy4KPiA+Pgo+ID4+IFN0aWxsLCB0aGlzIGlzIGEgcHJl dHR5IG1pbm9yIGlzc3VlLCBhbmQgb3ZlcmFsbCB0aGlzIHNlcmllcyBsb29rcwo+ID4+IHJlYXNv bmFibGUgdG8gbWUgYXQgYSBxdWljayBsb29rLgo+ID4KPiA+IFllYWgsIEkgYWdyZWUgYW5kIHBl cnNvbmFsbHkgcHJlZmVyIGEpLCBidXQgZ2l2ZW4gdGhlIGxpdHRsZSBmZWVkYmFjawo+ID4gdGhh dCBJIGhhdmUgZ290dGVuIHNvIGZhciBvbiB0aGUgcmVmYWN0b3JpbmcsIEknbSBzdGFydGluZyB0 byBjb25zaWRlcgo+ID4gZm9yZ2V0dGluZyBhYm91dCB0aGUgcGVyLXVzZXIgY2xrIHN0cnVjdCBh bmQgZ28gaW5zdGVhZCB3aXRoIGEKPiA+IHJlcXVlc3QtYmFzZWQgQVBJIHNpbWlsYXIgdG8gdGhh dCBvZiBwbV9xb3MsIGZvciBzZXR0aW5nIGZsb29yIGFuZAo+ID4gY2VpbGluZyBmcmVxdWVuY2ll cy4KPiAKPiBJIHdvdWxkIG9idmlvdXNseSBlbmNvdXJhZ2UgeW91IHRvIGNvbnRpbnVlIHB1c2hp bmcgZm9yIHRoaXMgZmVhdHVyZSwgCj4gYWx0aG91Z2ggSSB1bmRlcnN0YW5kIGl0IGNhbiBiZSBk aWZmaWN1bHQgdG8gYmUgbW90aXZhdGVkIHdoZW4gdGhlIAo+IHBhdGNoZXMgZG9uJ3QgZ2V0IG11 Y2ggZmVlZGJhY2suCgpTb3JyeSBmb3Igbm90IGdpdmluZyB0aGlzIG1vcmUgb2YgYSBsb29rLiBN eSBzZWNyZXQgcGxhbiB3YXMgdG8gcmV2aWV3CnRoaXMgaW4gbW9yZSBkZXRhaWwgb25jZSB3ZSdy ZSBjbG9zZXIgdG8gdGhlIG1lcmdlIHdpbmRvdyBhbmQsIGlmCmFwcHJvcHJpYXRlLCBhcHBseSBp dCBhcyBvbmUgb2YgdGhlIGZpcnN0IHRoaW5ncyBzaW5jZSB0aGlzIGlzIGJvdW5kIHRvCmNhdXNl IHNvbWUgYnJlYWthZ2UgZWFybHkgb24gd2hpY2ggbmVlZHMgdG8gYmUgY2F1Z2h0IGFuZCBmaXhl ZCB1cApxdWlja2x5LgoKV2UgYXJlIHZlcnkgY2xvc2UgdG8gdGhlIG1lcmdlIHdpbmRvdyBzbyBJ IHdpbGwgdHJ5IHRvIGdldCB0byB0aGlzIGxhdGVyCnRoaXMgd2Vlay4gSSBsaWtlIHRoZSBwZXIt dXNlciBzdHJ1Y3QgY2xrIG15c2VsZiBhbmQgSSBob3BlIHdlIGNhbiBnZXQKdGhpcyBpbiBmb3Ig My4xOC4KClJlZ2FyZHMsCk1pa2UKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX18KTGludXhwcGMtZGV2IG1haWxpbmcgbGlzdApMaW51eHBwYy1kZXZAbGlzdHMu b3psYWJzLm9yZwpodHRwczovL2xpc3RzLm96bGFicy5vcmcvbGlzdGluZm8vbGludXhwcGMtZGV2 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pa0-f48.google.com (mail-pa0-f48.google.com [209.85.220.48]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id BB8D61A021A for ; Fri, 1 Aug 2014 04:54:52 +1000 (EST) Received: by mail-pa0-f48.google.com with SMTP id et14so4173787pad.7 for ; Thu, 31 Jul 2014 11:54:50 -0700 (PDT) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 To: Stephen Warren , "Tomeu Vizoso" From: Mike Turquette In-Reply-To: <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> <53DA8218.7070300@wwwdotorg.org> Message-ID: <20140731185444.4463.35237@quantum> Subject: Re: [PATCH v4 6/6] clk: Add floor and ceiling constraints to clock rates Date: Thu, 31 Jul 2014 11:54:44 -0700 Cc: Ulf Hansson , Prashant Gaikwad , "Jason Cooper , Andrew Lunn , Sebastian Hesselbarth , Russell King , Shawn Guo , Sascha Hauer , David Brown , Daniel Walker , Bryan Huntsman , Paul Walmsley , Tony Lindgren , =?utf-8?q?=22_Beno=C3=AEt_Cousson?= , Kevin Hilman" , Tomasz Figa , alsa-devel@alsa-project.org, Jaroslav Kysela , Thierry Reding , Paul Mackerras , Sylwester Nawrocki , linux-arch@vger.kernel.org, Boris Brezillon , linux-samsung-soc@vger.kernel.org, Kukjin Kim , =?utf-8?b?IiBFbWlsaW8gTMOzcGV6?= , patches@opensource.wolfsonmicro.com, Michal Simek , Takashi Iwai , Santosh Shilimkar , Anatolij Gustschin , Dinh Nguyen , linux-media@vger.kernel.org, Arnd Bergmann , linux-arm-msm@vger.kernel.org, spear-devel@list.st.com, Mark Brown , linux-rpi-kernel@lists.infradead.org, Ben Dooks , linux-tegra@vger.kernel.org, linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Barry Song , Peter De Schrijver , Liam Girdwood , linux-kernel@vger.kernel.org, rabin@rab.in, Kyungmin Park , Viresh Kumar , Maxime Ripard , 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: , Quoting Stephen Warren (2014-07-31 10:51:20) > 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. Sorry for not giving this more of a look. My secret plan was to review this in more detail once we're closer to the merge window and, if appropriate, apply it as one of the first things since this is bound to cause some breakage early on which needs to be caught and fixed up quickly. We are very close to the merge window so I will try to get to this later this week. I like the per-user struct clk myself and I hope we can get this in for 3.18. Regards, Mike From mboxrd@z Thu Jan 1 00:00:00 1970 From: mturquette@linaro.org (Mike Turquette) Date: Thu, 31 Jul 2014 11:54:44 -0700 Subject: [PATCH v4 6/6] clk: Add floor and ceiling constraints to clock rates In-Reply-To: <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> <53DA8218.7070300@wwwdotorg.org> Message-ID: <20140731185444.4463.35237@quantum> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Quoting Stephen Warren (2014-07-31 10:51:20) > 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. Sorry for not giving this more of a look. My secret plan was to review this in more detail once we're closer to the merge window and, if appropriate, apply it as one of the first things since this is bound to cause some breakage early on which needs to be caught and fixed up quickly. We are very close to the merge window so I will try to get to this later this week. I like the per-user struct clk myself and I hope we can get this in for 3.18. Regards, Mike