From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Lezcano Subject: Re: [PATCH 05/18] cpuidle: make a single register function for all Date: Wed, 10 Apr 2013 20:04:22 +0200 Message-ID: <5165A9A6.70905@linaro.org> References: <1365603743-5618-1-git-send-email-daniel.lezcano@linaro.org> <1365603743-5618-6-git-send-email-daniel.lezcano@linaro.org> <20130410165509.GK13524@lunn.ch> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <20130410165509.GK13524@lunn.ch> 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: Andrew Lunn Cc: khilman@deeprootsystems.com, linus.walleij@linaro.org, nsekhar@ti.com, josephl@nvidia.com, kgene.kim@samsung.com, patches@linaro.org, magnus.damm@gmail.com, tony@atomide.com, plagnioj@jcrosoft.com, linaro-kernel@lists.linaro.org, jason@lakedaemon.net, swarren@wwwdotorg.org, nicolas.ferre@atmel.com, rob.herring@calxeda.com, rjw@sisk.pl, horms@verge.net.au, linux@maxim.org.za, linux-arm-kernel@lists.infradead.org, deepthi@linux.vnet.ibm.com, jkosina@suse.cz, linux-pm@vger.kernel.org, lethal@linux-sh.org, santosh.shilimkar@ti.com, kernel@pengutronix.de List-Id: linux-pm@vger.kernel.org T24gMDQvMTAvMjAxMyAwNjo1NSBQTSwgQW5kcmV3IEx1bm4gd3JvdGU6Cj4+ICsvKioKPj4gKyAq IGNwdWlkbGVfcmVnaXN0ZXI6IHJlZ2lzdGVycyB0aGUgZHJpdmVyIGFuZCB0aGUgY3B1IGRldmlj ZXMgd2l0aCB0aGUKPj4gKyAqIGNvdXBsZWRfY3B1cyBwYXNzZWQgYXMgcGFyYW1ldGVyLiBUaGlz IGZ1bmN0aW9uIGlzIHVzZWQgZm9yIGFsbCBjb21tb24KPj4gKyAqIGluaXRpYWxpemF0aW9uIHBh dHRlcm4gdGhlcmUgYXJlIGluIHRoZSBhcmNoIHNwZWNpZmljIGRyaXZlcnMuIFRoZQo+PiArICog ZGV2aWNlcyBpcyBnbG9iYWxseSBkZWZpbmVkIGluIHRoaXMgZmlsZS4KPj4gKyAqCj4+ICsgKiBA ZHJ2ICAgICAgICAgOiBhIHZhbGlkIHBvaW50ZXIgdG8gYSBzdHJ1Y3QgY3B1aWRsZV9kcml2ZXIK Pj4gKyAqIEBjb3VwbGVkX2NwdXM6IGEgY3B1bWFzayBmb3IgdGhlIGNvdXBsZWQgc3RhdGVzCj4+ ICsgKgo+PiArICogUmV0dXJucyAwIG9uIHN1Y2Nlc3MsIDwgMCBvdGhlcndpc2UKPj4gKyAqLwo+ PiAraW50IGNwdWlkbGVfcmVnaXN0ZXIoc3RydWN0IGNwdWlkbGVfZHJpdmVyICpkcnYsCj4+ICsJ CSAgICAgY29uc3Qgc3RydWN0IGNwdW1hc2sgKmNvbnN0IGNvdXBsZWRfY3B1cykKPj4gK3sKPj4g KwlpbnQgcmV0LCBjcHU7Cj4+ICsJc3RydWN0IGNwdWlkbGVfZGV2aWNlICpkZXZpY2U7Cj4+ICsK Pj4gKwlyZXQgPSBjcHVpZGxlX3JlZ2lzdGVyX2RyaXZlcihkcnYpOwo+PiArCWlmIChyZXQpIHsK Pj4gKwkJcHJpbnRrKEtFUk5fRVJSICJmYWlsZWQgdG8gcmVnaXN0ZXIgY3B1aWRsZSBkcml2ZXJc biIpOwo+IAo+IHByX2VycigpCgpPay4KCj4+ICsJCXJldHVybiByZXQ7Cj4+ICsJfQo+PiArCj4+ ICsJZm9yX2VhY2hfcG9zc2libGVfY3B1KGNwdSkgewo+PiArCQlkZXZpY2UgPSAmcGVyX2NwdShj cHVpZGxlX2RldiwgY3B1KTsKPj4gKwkJZGV2aWNlLT5jcHUgPSBjcHU7Cj4+ICsjaWZkZWYgQ09O RklHX0FSQ0hfTkVFRFNfQ1BVX0lETEVfQ09VUExFRAo+PiArCQlkZXZpY2UtPmNvdXBsZWRfY3B1 cyA9ICpjb3VwbGVkX2NwdXM7Cj4+ICsjZW5kaWYKPj4gKwkJcmV0ID0gY3B1aWRsZV9yZWdpc3Rl cl9kZXZpY2UoZGV2aWNlKTsKPj4gKwkJaWYgKCFyZXQpCj4+ICsJCQljb250aW51ZTsKPj4gKwo+ PiArCQlwcmludGsoS0VSTl9FUlIgIkZhaWxlZCB0byByZWdpc3RlciBjcHVpZGxlICIKPj4gKwkJ ICAgICAgICJkZXZpY2UgZm9yIGNwdSVkXG4iLCBjcHUpOwo+IAo+IHByX2VycigpIGFuZCBkb24n dCBzcGxpdCB0aGUgbWVzc2FnZSBvdmVyIHR3byBsaW5lcywgaXQgbWFrZXMgaXQKPiBoYXJkZXIg Zm9yIHNvbWVib2R5IHRvIGZpbmQgd2l0aAo+IAo+IGdyZXAgLXIgIkZhaWxlZCB0byByZWdpc3Rl ciBjcHVpZGxlIGRldmljZSBmb3IgY3B1IiAqCgpPayBpZiB0aGUgbGluZSBsZW5ndGggaXMgdW5k ZXIgODAgY2hhcnMuCgo+PiArCQljcHVpZGxlX3VucmVnaXN0ZXIoZHJ2KTsKPj4gKwkJYnJlYWs7 Cj4+ICsJfQo+PiArCj4+ICsJcmV0dXJuIDA7Cj4gCj4gWW91IHNob3VsZCByZXR1cm4gYW4gZXJy b3IgY29kZSwgc28gdGhhdCB0aGUgY2FsbGVyIGNhbiBhbHNvIHJldHVybiBhbgo+IGVycm9yIGNv ZGUuIElmIHlvdSBsb29rIGF0IGNwdWlkbGUta2lya3dvb2QgYW5kIGNwdWlkbGUtY2FseGVkYSwg YW5kCj4gbWF5YmUgb3RoZXJzLCBpZiB0aGUgcmVnaXN0cmF0aW9uIGZhaWxzLCB0aGUgcHJvYmUg ZnVuY3Rpb24gcmV0dXJucyBhbgo+IGVycm9yIGNvZGUsIGFzIGl0IHNob3VsZC4gV2l0aCB5b3Vy IGNoYW5nZSwgaXRzIGFsd2F5cyBnb2luZyB0byByZXR1cm4KPiAwLCBldmVuIGlmIGl0IGZhaWxz LgoKUmlnaHQsIHJpZ2h0IDopCgppdCBzaG91bGQgYmUgJ3JldHVybiByZXQ7JwoKVGhhbmtzIGZv ciBwb2ludGluZyB0aGlzLgoKICAtLSBEYW5pZWwKCgotLSAKIDxodHRwOi8vd3d3LmxpbmFyby5v cmcvPiBMaW5hcm8ub3JnIOKUgiBPcGVuIHNvdXJjZSBzb2Z0d2FyZSBmb3IgQVJNIFNvQ3MKCkZv bGxvdyBMaW5hcm86ICA8aHR0cDovL3d3dy5mYWNlYm9vay5jb20vcGFnZXMvTGluYXJvPiBGYWNl Ym9vayB8CjxodHRwOi8vdHdpdHRlci5jb20vIyEvbGluYXJvb3JnPiBUd2l0dGVyIHwKPGh0dHA6 Ly93d3cubGluYXJvLm9yZy9saW5hcm8tYmxvZy8+IEJsb2cKCgpfX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fXwpsaW51eC1hcm0ta2VybmVsIG1haWxpbmcgbGlz dApsaW51eC1hcm0ta2VybmVsQGxpc3RzLmluZnJhZGVhZC5vcmcKaHR0cDovL2xpc3RzLmluZnJh ZGVhZC5vcmcvbWFpbG1hbi9saXN0aW5mby9saW51eC1hcm0ta2VybmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 From: daniel.lezcano@linaro.org (Daniel Lezcano) Date: Wed, 10 Apr 2013 20:04:22 +0200 Subject: [PATCH 05/18] cpuidle: make a single register function for all In-Reply-To: <20130410165509.GK13524@lunn.ch> References: <1365603743-5618-1-git-send-email-daniel.lezcano@linaro.org> <1365603743-5618-6-git-send-email-daniel.lezcano@linaro.org> <20130410165509.GK13524@lunn.ch> Message-ID: <5165A9A6.70905@linaro.org> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 04/10/2013 06:55 PM, Andrew Lunn wrote: >> +/** >> + * cpuidle_register: registers the driver and the cpu devices with the >> + * coupled_cpus passed as parameter. This function is used for all common >> + * initialization pattern there are in the arch specific drivers. The >> + * devices is globally defined in this file. >> + * >> + * @drv : a valid pointer to a struct cpuidle_driver >> + * @coupled_cpus: a cpumask for the coupled states >> + * >> + * Returns 0 on success, < 0 otherwise >> + */ >> +int cpuidle_register(struct cpuidle_driver *drv, >> + const struct cpumask *const coupled_cpus) >> +{ >> + int ret, cpu; >> + struct cpuidle_device *device; >> + >> + ret = cpuidle_register_driver(drv); >> + if (ret) { >> + printk(KERN_ERR "failed to register cpuidle driver\n"); > > pr_err() Ok. >> + return ret; >> + } >> + >> + for_each_possible_cpu(cpu) { >> + device = &per_cpu(cpuidle_dev, cpu); >> + device->cpu = cpu; >> +#ifdef CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED >> + device->coupled_cpus = *coupled_cpus; >> +#endif >> + ret = cpuidle_register_device(device); >> + if (!ret) >> + continue; >> + >> + printk(KERN_ERR "Failed to register cpuidle " >> + "device for cpu%d\n", cpu); > > pr_err() and don't split the message over two lines, it makes it > harder for somebody to find with > > grep -r "Failed to register cpuidle device for cpu" * Ok if the line length is under 80 chars. >> + cpuidle_unregister(drv); >> + break; >> + } >> + >> + return 0; > > You should return an error code, so that the caller can also return an > error code. If you look at cpuidle-kirkwood and cpuidle-calxeda, and > maybe others, if the registration fails, the probe function returns an > error code, as it should. With your change, its always going to return > 0, even if it fails. Right, right :) it should be 'return ret;' Thanks for pointing this. -- Daniel -- Linaro.org ? Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog