From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Lezcano Subject: Re: [RFC patch 1/2] ARM: at91: cpuidle: encapsulate the standby code Date: Mon, 15 Apr 2013 16:13:59 +0200 Message-ID: <516C0B27.9080406@linaro.org> References: <1366032598-23053-1-git-send-email-daniel.lezcano@linaro.org> <1366032598-23053-2-git-send-email-daniel.lezcano@linaro.org> <516C069E.2040205@atmel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <516C069E.2040205@atmel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: Nicolas Ferre Cc: linaro-kernel@lists.linaro.org, linux-pm@vger.kernel.org, rjw@sisk.pl, plagnioj@jcrosoft.com, linux@maxim.org.za, linux-arm-kernel@lists.infradead.org List-Id: linux-pm@vger.kernel.org T24gMDQvMTUvMjAxMyAwMzo1NCBQTSwgTmljb2xhcyBGZXJyZSB3cm90ZToKPiBIaSBEYW5pZWws Cj4gCj4gT24gMDQvMTUvMjAxMyAwMzoyOSBQTSwgRGFuaWVsIExlemNhbm8gOgo+PiBJbiBvcmRl ciB0byBzcGxpdCB0aGUgcG0gY29kZSBmcm9tIHRoZSBjcHVpZGxlIGRyaXZlciwgYWRkIGFuIG9w cyBmb3IgdGhlCj4+IHN0YW5kYnkgZnVuY3Rpb24gd2hpY2ggd2lsbCBiZSBpbml0aWFsaXplZCBi eSB0aGUgcG0gaW5pdCBmdW5jdGlvbnMgZGlyZWN0bHksCj4+IHRodXMgbm8gbmVlZCBvZiB0aGUg U29DIHNwZWNpZmljIGhlYWRlcnMuCj4+Cj4+IENsZWFudXAgYWxzbyB0aGUgaGVhZGVycyBpbmNs dWRlZCBpbiB0aGlzIGZpbGUgYXMgdGhleSBhcmUgbm8gbG9uZ2VyIG5lZWRlZC4KPj4KPj4gU2ln bmVkLW9mZi1ieTogRGFuaWVsIExlemNhbm8gPGRhbmllbC5sZXpjYW5vQGxpbmFyby5vcmc+Cj4+ IC0tLQo+PiAgYXJjaC9hcm0vbWFjaC1hdDkxL2NwdWlkbGUuYyB8ICAgMTkgKysrKy0tLS0tLS0t LS0tLS0tLQo+PiAgYXJjaC9hcm0vbWFjaC1hdDkxL3BtLmMgICAgICB8ICAgIDggKysrKysrKy0K Pj4gIDIgZmlsZXMgY2hhbmdlZCwgMTEgaW5zZXJ0aW9ucygrKSwgMTYgZGVsZXRpb25zKC0pCj4+ Cj4+IGRpZmYgLS1naXQgYS9hcmNoL2FybS9tYWNoLWF0OTEvY3B1aWRsZS5jIGIvYXJjaC9hcm0v bWFjaC1hdDkxL2NwdWlkbGUuYwo+PiBpbmRleCA0OGYxMjI4Li5iMmJlYzkyIDEwMDY0NAo+PiAt LS0gYS9hcmNoL2FybS9tYWNoLWF0OTEvY3B1aWRsZS5jCj4+ICsrKyBiL2FyY2gvYXJtL21hY2gt YXQ5MS9jcHVpZGxlLmMKPj4gQEAgLTEzLDMyICsxMywyMSBAQAo+PiAgICogIzIgd2FpdC1mb3It aW50ZXJydXB0IGFuZCBSQU0gc2VsZiByZWZyZXNoCj4+ICAgKi8KPj4gIAo+PiAtI2luY2x1ZGUg PGxpbnV4L2tlcm5lbC5oPgo+PiArI2luY2x1ZGUgPGxpbnV4L21vZHVsZS5oPgo+PiAgI2luY2x1 ZGUgPGxpbnV4L2luaXQuaD4KPj4gLSNpbmNsdWRlIDxsaW51eC9wbGF0Zm9ybV9kZXZpY2UuaD4K Pj4gICNpbmNsdWRlIDxsaW51eC9jcHVpZGxlLmg+Cj4+IC0jaW5jbHVkZSA8bGludXgvaW8uaD4K Pj4gLSNpbmNsdWRlIDxsaW51eC9leHBvcnQuaD4KPj4gLSNpbmNsdWRlIDxhc20vcHJvYy1mbnMu aD4KPj4gICNpbmNsdWRlIDxhc20vY3B1aWRsZS5oPgo+PiAtI2luY2x1ZGUgPG1hY2gvY3B1Lmg+ Cj4+IC0KPj4gLSNpbmNsdWRlICJwbS5oIgo+PiAgCj4+ICAjZGVmaW5lIEFUOTFfTUFYX1NUQVRF UwkyCj4+ICAKPj4gK2V4dGVybiB2b2lkICgqYXQ5MV9zdGFuZGJ5X29wcykodm9pZCk7Cj4+ICsK Pj4gIC8qIEFjdHVhbCBjb2RlIHRoYXQgcHV0cyB0aGUgU29DIGluIGRpZmZlcmVudCBpZGxlIHN0 YXRlcyAqLwo+PiAgc3RhdGljIGludCBhdDkxX2VudGVyX2lkbGUoc3RydWN0IGNwdWlkbGVfZGV2 aWNlICpkZXYsCj4+ICAJCQlzdHJ1Y3QgY3B1aWRsZV9kcml2ZXIgKmRydiwKPj4gIAkJCSAgICAg ICBpbnQgaW5kZXgpCj4+ICB7Cj4+IC0JaWYgKGNwdV9pc19hdDkxcm05MjAwKCkpCj4+IC0JCWF0 OTFybTkyMDBfc3RhbmRieSgpOwo+PiAtCWVsc2UgaWYgKGNwdV9pc19hdDkxc2FtOWc0NSgpKQo+ PiAtCQlhdDkxc2FtOWc0NV9zdGFuZGJ5KCk7Cj4+IC0JZWxzZQo+PiAtCQlhdDkxc2FtOV9zdGFu ZGJ5KCk7Cj4+IC0KPj4gKwlhdDkxX3N0YW5kYnlfb3BzKCk7Cj4+ICAJcmV0dXJuIGluZGV4Owo+ PiAgfQo+PiAgCj4+IGRpZmYgLS1naXQgYS9hcmNoL2FybS9tYWNoLWF0OTEvcG0uYyBiL2FyY2gv YXJtL21hY2gtYXQ5MS9wbS5jCj4+IGluZGV4IDczZjFmMjUuLmY0NTZlODYgMTAwNjQ0Cj4+IC0t LSBhL2FyY2gvYXJtL21hY2gtYXQ5MS9wbS5jCj4+ICsrKyBiL2FyY2gvYXJtL21hY2gtYXQ5MS9w bS5jCj4+IEBAIC0zOSw2ICszOSw4IEBACj4+ICAjaW5jbHVkZSAiYXQ5MV9yc3RjLmgiCj4+ICAj aW5jbHVkZSAiYXQ5MV9zaGR3Yy5oIgo+PiAgCj4+ICt2b2lkICgqYXQ5MV9zdGFuZGJ5X29wcyko dm9pZCk7Cj4gCj4gSXMgdGhpcyBhIGNvbW1vbiBwYXR0ZXJuIHRvIGhhdmUgc3VjaCBhIGZsb2F0 aW5nIGZ1bmN0aW9uIHBvaW50ZXIgaW4gdGhlCj4gcG0gY29kZT8KCldlbGwsIGFscmVhZHkgc2Vl biBidXQgSSBhZ3JlZSBpdCBpcyBub3QgcmVhbGx5IG5pY2UuCgpUaGUgaWRlYSBJIGhhZCB3YXMg dG8gY29udmVydCBsaXR0bGUgYnkgbGl0dGxlIGFsbCB0aGVzZSBwbSBmdW5jdGlvbnMKaW50byBv cHMsIHRoZW4gb3JkZXIsIGdyb3VwIGFuZCB1c2UgdGhlbSB0byBpbml0aWFsaXplIHRoZSBjcHVp ZGxlCmRyaXZlcnMgdGhyb3VnaCBhIHNpbmdsZSBBUk0gZHJpdmVyLiBUaGUgY29ycmVjdCBvcmRl ciB3b3VsZCBoYXZlIGJlZW4KdG8gY29udmVydCBmaXJzdCB0aGVzZSB0byBvcHMgdGhlbiBtb3Zl IHRoZSBkcml2ZXIgYnV0IHRoZXJlIGFyZSBzbyBtYW55CmRyaXZlcnMsIHNvIG1hbnkgY29kZSwg SSBkb24ndCBrbm93IHdoZXJlIGRvIEkgc3RhcnQuCgpEbyB5b3UgaGF2ZSBhIHN1Z2dlc3Rpb24g PwoKPj4gKwo+PiAgc3RhdGljIHZvaWQgX19pbml0IHNob3dfcmVzZXRfc3RhdHVzKHZvaWQpCj4+ ICB7Cj4+ICAJc3RhdGljIGNoYXIgcmVzZXRbXSBfX2luaXRkYXRhID0gInJlc2V0IjsKPj4gQEAg LTMyMSw4ICszMjMsMTIgQEAgc3RhdGljIGludCBfX2luaXQgYXQ5MV9wbV9pbml0KHZvaWQpCj4+ ICAJcHJfaW5mbygiQVQ5MTogUG93ZXIgTWFuYWdlbWVudCVzXG4iLCAoc2xvd19jbG9jayA/ICIg KHdpdGggc2xvdyBjbG9jayBtb2RlKSIgOiAiIikpOwo+PiAgCj4+ICAJLyogQVQ5MVJNOTIwMCBT RFJBTSBsb3ctcG93ZXIgbW9kZSBjYW5ub3QgYmUgdXNlZCB3aXRoIHNlbGYtcmVmcmVzaC4gKi8K Pj4gLQlpZiAoY3B1X2lzX2F0OTFybTkyMDAoKSkKPj4gKwlpZiAoY3B1X2lzX2F0OTFybTkyMDAo KSkgewo+PiAgCQlhdDkxX3JhbWNfd3JpdGUoMCwgQVQ5MVJNOTIwMF9TRFJBTUNfTFBSLCAwKTsK Pj4gKwkJYXQ5MV9zdGFuZGJ5X29wcyA9IGF0OTFybTkyMDBfc3RhbmRieTsKPj4gKwl9IGVsc2Ug aWYgKGNwdV9pc19hdDkxc2FtOWc0NSgpKQo+IAo+IENvZGluZ1N0eWxlOiBlbmRpbmcgInsiIGlz IG1pc3NpbmcuCj4gCj4+ICsJCWF0OTFfc3RhbmRieV9vcHM9IGF0OTFzYW05ZzQ1X3N0YW5kYnk7 Cj4gCj4gQ29kaW5nU3R5bGU6ICIgIiBpcyBtaXNzaW5nCj4gCj4+ICsJZWxzZSBhdDkxX3N0YW5k Ynlfb3BzID0gYXQ5MXNhbTlfc3RhbmRieTsKPiAKPiBDb2RpbmdTdHlsZTogbm90IG9uIHRoZSBz YW1lIGxpbmUgKyAie30iIG1pc3NpbmcKPiAKPj4gIAo+PiAgCXN1c3BlbmRfc2V0X29wcygmYXQ5 MV9wbV9vcHMpOwo+IAo+IEkgYW0gaW4gZmF2b3IgZm9yIHRoZSBtb3ZlLiAKCk9rLCBjb29sLgoK PiBCdXQgcGxlYXNlIHJld3JpdGUgYSBuZXcgc2VyaWVzLgoKWWVzLCBzdXJlLCB0aGF0IHdhcyBh IGRyYWZ0LgoKVGhhbmtzIGZvciB0aGUgcmV2aWV3LgoKICAtLSBEYW5pZWwKCgotLSAKIDxodHRw Oi8vd3d3LmxpbmFyby5vcmcvPiBMaW5hcm8ub3JnIOKUgiBPcGVuIHNvdXJjZSBzb2Z0d2FyZSBm b3IgQVJNIFNvQ3MKCkZvbGxvdyBMaW5hcm86ICA8aHR0cDovL3d3dy5mYWNlYm9vay5jb20vcGFn ZXMvTGluYXJvPiBGYWNlYm9vayB8CjxodHRwOi8vdHdpdHRlci5jb20vIyEvbGluYXJvb3JnPiBU d2l0dGVyIHwKPGh0dHA6Ly93d3cubGluYXJvLm9yZy9saW5hcm8tYmxvZy8+IEJsb2cKCgpfX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpsaW51eC1hcm0ta2Vy bmVsIG1haWxpbmcgbGlzdApsaW51eC1hcm0ta2VybmVsQGxpc3RzLmluZnJhZGVhZC5vcmcKaHR0 cDovL2xpc3RzLmluZnJhZGVhZC5vcmcvbWFpbG1hbi9saXN0aW5mby9saW51eC1hcm0ta2VybmVs Cg== From mboxrd@z Thu Jan 1 00:00:00 1970 From: daniel.lezcano@linaro.org (Daniel Lezcano) Date: Mon, 15 Apr 2013 16:13:59 +0200 Subject: [RFC patch 1/2] ARM: at91: cpuidle: encapsulate the standby code In-Reply-To: <516C069E.2040205@atmel.com> References: <1366032598-23053-1-git-send-email-daniel.lezcano@linaro.org> <1366032598-23053-2-git-send-email-daniel.lezcano@linaro.org> <516C069E.2040205@atmel.com> Message-ID: <516C0B27.9080406@linaro.org> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 04/15/2013 03:54 PM, Nicolas Ferre wrote: > Hi Daniel, > > On 04/15/2013 03:29 PM, Daniel Lezcano : >> In order to split the pm code from the cpuidle driver, add an ops for the >> standby function which will be initialized by the pm init functions directly, >> thus no need of the SoC specific headers. >> >> Cleanup also the headers included in this file as they are no longer needed. >> >> Signed-off-by: Daniel Lezcano >> --- >> arch/arm/mach-at91/cpuidle.c | 19 ++++--------------- >> arch/arm/mach-at91/pm.c | 8 +++++++- >> 2 files changed, 11 insertions(+), 16 deletions(-) >> >> diff --git a/arch/arm/mach-at91/cpuidle.c b/arch/arm/mach-at91/cpuidle.c >> index 48f1228..b2bec92 100644 >> --- a/arch/arm/mach-at91/cpuidle.c >> +++ b/arch/arm/mach-at91/cpuidle.c >> @@ -13,32 +13,21 @@ >> * #2 wait-for-interrupt and RAM self refresh >> */ >> >> -#include >> +#include >> #include >> -#include >> #include >> -#include >> -#include >> -#include >> #include >> -#include >> - >> -#include "pm.h" >> >> #define AT91_MAX_STATES 2 >> >> +extern void (*at91_standby_ops)(void); >> + >> /* Actual code that puts the SoC in different idle states */ >> static int at91_enter_idle(struct cpuidle_device *dev, >> struct cpuidle_driver *drv, >> int index) >> { >> - if (cpu_is_at91rm9200()) >> - at91rm9200_standby(); >> - else if (cpu_is_at91sam9g45()) >> - at91sam9g45_standby(); >> - else >> - at91sam9_standby(); >> - >> + at91_standby_ops(); >> return index; >> } >> >> diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c >> index 73f1f25..f456e86 100644 >> --- a/arch/arm/mach-at91/pm.c >> +++ b/arch/arm/mach-at91/pm.c >> @@ -39,6 +39,8 @@ >> #include "at91_rstc.h" >> #include "at91_shdwc.h" >> >> +void (*at91_standby_ops)(void); > > Is this a common pattern to have such a floating function pointer in the > pm code? Well, already seen but I agree it is not really nice. The idea I had was to convert little by little all these pm functions into ops, then order, group and use them to initialize the cpuidle drivers through a single ARM driver. The correct order would have been to convert first these to ops then move the driver but there are so many drivers, so many code, I don't know where do I start. Do you have a suggestion ? >> + >> static void __init show_reset_status(void) >> { >> static char reset[] __initdata = "reset"; >> @@ -321,8 +323,12 @@ static int __init at91_pm_init(void) >> pr_info("AT91: Power Management%s\n", (slow_clock ? " (with slow clock mode)" : "")); >> >> /* AT91RM9200 SDRAM low-power mode cannot be used with self-refresh. */ >> - if (cpu_is_at91rm9200()) >> + if (cpu_is_at91rm9200()) { >> at91_ramc_write(0, AT91RM9200_SDRAMC_LPR, 0); >> + at91_standby_ops = at91rm9200_standby; >> + } else if (cpu_is_at91sam9g45()) > > CodingStyle: ending "{" is missing. > >> + at91_standby_ops= at91sam9g45_standby; > > CodingStyle: " " is missing > >> + else at91_standby_ops = at91sam9_standby; > > CodingStyle: not on the same line + "{}" missing > >> >> suspend_set_ops(&at91_pm_ops); > > I am in favor for the move. Ok, cool. > But please rewrite a new series. Yes, sure, that was a draft. Thanks for the review. -- Daniel -- Linaro.org ? Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog