From patchwork Fri Apr 12 12:35:52 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Lezcano X-Patchwork-Id: 16098 Return-Path: X-Original-To: linaro@staging.patches.linaro.org Delivered-To: linaro@staging.patches.linaro.org Received: from mail-gh0-f197.google.com (mail-gh0-f197.google.com [209.85.160.197]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id CE74C26EB0 for ; Fri, 12 Apr 2013 12:37:08 +0000 (UTC) Received: by mail-gh0-f197.google.com with SMTP id r17sf3711966ghr.0 for ; Fri, 12 Apr 2013 05:36:45 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-received:mime-version:x-beenthere:x-received:received-spf :x-received:x-forwarded-to:x-forwarded-for:delivered-to:x-received :received-spf:x-received:from:to:cc:subject:date:message-id:x-mailer :in-reply-to:references:x-gm-message-state:x-original-sender :x-original-authentication-results:precedence:mailing-list:list-id :x-google-group-id:list-post:list-help:list-archive:list-unsubscribe; bh=xwVtqU50HvxIFCRu6a49uzr/vuJhOolj23PW40c/xhw=; b=NGti9qxksBFa6xOm3k8RkXjnexKPNcytGPBdxM3A51aeMBYz27p/ea3RtLYBZGuGOo guDapl0CVy1mgGV87uub4uHbOdylWX49nu4A00B7CDKAS7s/W1cmtkxIr578bMuEeNsV RTFvTJ0I9t4Z7FHOnqC0CREesauGkCSFWMMUBsEgl18mdwMZba+dZkYVY1s0tJ1Wbqf8 WFWIWLsmnSkiktapz8NRxPJr5paKSGBvrTapkMSRQTxzVKcuCcnHTPLVuoh1YU/ZuJN0 xNt/nvR9SmF7ohriFhg3mg8epJ7WXaSr9KrAnNRoQzs1qdZg5mmZGeAOOEXAHuxBGbGB GxHQ== X-Received: by 10.224.65.6 with SMTP id g6mr6116176qai.4.1365770205208; Fri, 12 Apr 2013 05:36:45 -0700 (PDT) MIME-Version: 1.0 X-BeenThere: patchwork-forward@linaro.org Received: by 10.49.39.138 with SMTP id p10ls1266437qek.16.gmail; Fri, 12 Apr 2013 05:36:44 -0700 (PDT) X-Received: by 10.220.248.200 with SMTP id mh8mr8202124vcb.51.1365770204881; Fri, 12 Apr 2013 05:36:44 -0700 (PDT) Received: from mail-vc0-f170.google.com (mail-vc0-f170.google.com [209.85.220.170]) by mx.google.com with ESMTPS id ec8si6363911vdb.46.2013.04.12.05.36.44 (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Fri, 12 Apr 2013 05:36:44 -0700 (PDT) Received-SPF: neutral (google.com: 209.85.220.170 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) client-ip=209.85.220.170; Received: by mail-vc0-f170.google.com with SMTP id lf10so2161005vcb.15 for ; Fri, 12 Apr 2013 05:36:44 -0700 (PDT) X-Received: by 10.58.205.179 with SMTP id lh19mr8170328vec.7.1365770204721; Fri, 12 Apr 2013 05:36:44 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patches@linaro.org Received: by 10.58.127.98 with SMTP id nf2csp64082veb; Fri, 12 Apr 2013 05:36:44 -0700 (PDT) X-Received: by 10.194.110.69 with SMTP id hy5mr2428945wjb.1.1365770189510; Fri, 12 Apr 2013 05:36:29 -0700 (PDT) Received: from mail-wg0-f46.google.com (mail-wg0-f46.google.com [74.125.82.46]) by mx.google.com with ESMTPS id bl4si3029980wjb.29.2013.04.12.05.36.29 (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Fri, 12 Apr 2013 05:36:29 -0700 (PDT) Received-SPF: neutral (google.com: 74.125.82.46 is neither permitted nor denied by best guess record for domain of daniel.lezcano@linaro.org) client-ip=74.125.82.46; Received: by mail-wg0-f46.google.com with SMTP id e11so462402wgh.13 for ; Fri, 12 Apr 2013 05:36:29 -0700 (PDT) X-Received: by 10.180.36.48 with SMTP id n16mr3724217wij.30.1365770189008; Fri, 12 Apr 2013 05:36:29 -0700 (PDT) Received: from mai.home (AToulouse-654-1-349-85.w90-55.abo.wanadoo.fr. [90.55.188.85]) by mx.google.com with ESMTPS id ej8sm3043646wib.9.2013.04.12.05.36.25 (version=TLSv1.1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Fri, 12 Apr 2013 05:36:27 -0700 (PDT) From: Daniel Lezcano To: rjw@sisk.pl Cc: linus.walleij@linaro.org, jason@lakedaemon.net, andrew@lunn.ch, kernel@pengutronix.de, swarren@wwwdotorg.org, santosh.shilimkar@ti.com, nicolas.ferre@atmel.com, plagnioj@jcrosoft.com, linux@maxim.org.za, rob.herring@calxeda.com, nsekhar@ti.com, horms@verge.net.au, magnus.damm@gmail.com, deepthi@linux.vnet.ibm.com, lethal@linux-sh.org, jkosina@suse.cz, kgene.kim@samsung.com, khilman@deeprootsystems.com, tony@atomide.com, linux-pm@vger.kernel.org, patches@linaro.org, linux-arm-kernel@lists.infradead.org, linaro-kernel@lists.linaro.org, josephl@nvidia.com Subject: [V3 patch 06/19] cpuidle: make a single register function for all Date: Fri, 12 Apr 2013 14:35:52 +0200 Message-Id: <1365770165-27096-7-git-send-email-daniel.lezcano@linaro.org> X-Mailer: git-send-email 1.7.9.5 In-Reply-To: <1365770165-27096-1-git-send-email-daniel.lezcano@linaro.org> References: <1365770165-27096-1-git-send-email-daniel.lezcano@linaro.org> X-Gm-Message-State: ALoCoQnYZvnkuvW4xsSzrqMPBgSuu/WYO6LGNorG4dP8AuWYO5fXYLVfgqUFUCb5i3sXLYTKuGv5 X-Original-Sender: daniel.lezcano@linaro.org X-Original-Authentication-Results: mx.google.com; spf=neutral (google.com: 209.85.220.170 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Precedence: list Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org List-ID: X-Google-Group-Id: 836684582541 List-Post: , List-Help: , List-Archive: List-Unsubscribe: , The usual scheme to initialize a cpuidle driver on a SMP is: cpuidle_register_driver(drv); for_each_possible_cpu(cpu) { device = &per_cpu(cpuidle_dev, cpu); cpuidle_register_device(device); } This code is duplicated in each cpuidle driver. On UP systems, it is done this way: cpuidle_register_driver(drv); device = &per_cpu(cpuidle_dev, cpu); cpuidle_register_device(device); On UP, the macro 'for_each_cpu' does one iteration: #define for_each_cpu(cpu, mask) \ for ((cpu) = 0; (cpu) < 1; (cpu)++, (void)mask) Hence, the initialization loop is the same for UP than SMP. Beside, we saw different bugs / mis-initialization / return code unchecked in the different drivers, the code is duplicated including bugs. After fixing all these ones, it appears the initialization pattern is the same for everyone. Please note, some drivers are doing dev->state_count = drv->state_count. This is not necessary because it is done by the cpuidle_enable_device function in the cpuidle framework. This is true, until you have the same states for all your devices. Otherwise, the 'low level' API should be used instead with the specific initialization for the driver. Let's add a wrapper function doing this initialization with a cpumask parameter for the coupled idle states and use it for all the drivers. That will save a lot of LOC, consolidate the code, and the modifications in the future could be done in a single place. Another benefit is the consolidation of the cpuidle_device variable which is now in the cpuidle framework and no longer spread accross the different arch specific drivers. Signed-off-by: Daniel Lezcano --- Documentation/cpuidle/driver.txt | 6 ++++ drivers/cpuidle/cpuidle.c | 72 ++++++++++++++++++++++++++++++++++++++ include/linux/cpuidle.h | 9 +++-- 3 files changed, 85 insertions(+), 2 deletions(-) diff --git a/Documentation/cpuidle/driver.txt b/Documentation/cpuidle/driver.txt index 7a9e09e..1b0d81d 100644 --- a/Documentation/cpuidle/driver.txt +++ b/Documentation/cpuidle/driver.txt @@ -15,11 +15,17 @@ has mechanisms in place to support actual entry-exit into CPU idle states. cpuidle driver initializes the cpuidle_device structure for each CPU device and registers with cpuidle using cpuidle_register_device. +If all the idle states are the same, the wrapper function cpuidle_register +could be used instead. + It can also support the dynamic changes (like battery <-> AC), by using cpuidle_pause_and_lock, cpuidle_disable_device and cpuidle_enable_device, cpuidle_resume_and_unlock. Interfaces: +extern int cpuidle_register(struct cpuidle_driver *drv, + const struct cpumask *const coupled_cpus); +extern int cpuidle_unregister(struct cpuidle_driver *drv); extern int cpuidle_register_driver(struct cpuidle_driver *drv); extern void cpuidle_unregister_driver(struct cpuidle_driver *drv); extern int cpuidle_register_device(struct cpuidle_device *dev); diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c index 0da795b..49e8d30 100644 --- a/drivers/cpuidle/cpuidle.c +++ b/drivers/cpuidle/cpuidle.c @@ -24,6 +24,7 @@ #include "cpuidle.h" DEFINE_PER_CPU(struct cpuidle_device *, cpuidle_devices); +DEFINE_PER_CPU(struct cpuidle_device, cpuidle_dev); DEFINE_MUTEX(cpuidle_lock); LIST_HEAD(cpuidle_detected_devices); @@ -453,6 +454,77 @@ void cpuidle_unregister_device(struct cpuidle_device *dev) EXPORT_SYMBOL_GPL(cpuidle_unregister_device); +/* + * cpuidle_unregister: unregister a driver and the devices. This function + * can be used only if the driver has been previously registered through + * the cpuidle_register function. + * + * @drv: a valid pointer to a struct cpuidle_driver + */ +void cpuidle_unregister(struct cpuidle_driver *drv) +{ + int cpu; + struct cpuidle_device *device; + + for_each_possible_cpu(cpu) { + device = &per_cpu(cpuidle_dev, cpu); + cpuidle_unregister_device(device); + } + + cpuidle_unregister_driver(drv); +} +EXPORT_SYMBOL_GPL(cpuidle_unregister); + +/** + * 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) { + pr_err("failed to register cpuidle driver\n"); + return ret; + } + + for_each_possible_cpu(cpu) { + device = &per_cpu(cpuidle_dev, cpu); + device->cpu = cpu; + +#ifdef CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED + /* + * On multiplatform for ARM, the coupled idle states could + * enabled in the kernel even if the cpuidle driver does not + * use it. Note, coupled_cpus is a struct copy. + */ + if (coupled_cpus) + device->coupled_cpus = *coupled_cpus; +#endif + ret = cpuidle_register_device(device); + if (!ret) + continue; + + pr_err("Failed to register cpuidle device for cpu%d\n", cpu); + + cpuidle_unregister(drv); + break; + } + + return ret; +} +EXPORT_SYMBOL_GPL(cpuidle_register); + #ifdef CONFIG_SMP static void smp_callback(void *v) diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h index 79e3811..3c86faa 100644 --- a/include/linux/cpuidle.h +++ b/include/linux/cpuidle.h @@ -123,7 +123,9 @@ extern void cpuidle_driver_unref(void); extern void cpuidle_unregister_driver(struct cpuidle_driver *drv); extern int cpuidle_register_device(struct cpuidle_device *dev); extern void cpuidle_unregister_device(struct cpuidle_device *dev); - +extern int cpuidle_register(struct cpuidle_driver *drv, + const struct cpumask *const coupled_cpus); +extern void cpuidle_unregister(struct cpuidle_driver *drv); extern void cpuidle_pause_and_lock(void); extern void cpuidle_resume_and_unlock(void); extern void cpuidle_pause(void); @@ -148,7 +150,10 @@ static inline void cpuidle_unregister_driver(struct cpuidle_driver *drv) { } static inline int cpuidle_register_device(struct cpuidle_device *dev) {return -ENODEV; } static inline void cpuidle_unregister_device(struct cpuidle_device *dev) { } - +static inline int cpuidle_register(struct cpuidle_driver *drv, + const struct cpumask *const coupled_cpus) +{return -ENODEV; } +static inline void cpuidle_unregister(struct cpuidle_driver *drv) { } static inline void cpuidle_pause_and_lock(void) { } static inline void cpuidle_resume_and_unlock(void) { } static inline void cpuidle_pause(void) { }