From: "Rafael J. Wysocki" <rjw@sisk.pl>
To: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: linux-pm@vger.kernel.org, linaro-kernel@lists.linaro.org,
patches@linaro.org
Subject: Re: [PATCH 1/8] cpuidle: Remove governor module format
Date: Thu, 13 Jun 2013 00:53:53 +0200 [thread overview]
Message-ID: <1584034.uif9lZguq4@vostro.rjw.lan> (raw)
In-Reply-To: <1371042535-22630-1-git-send-email-daniel.lezcano@linaro.org>
On Wednesday, June 12, 2013 03:08:48 PM Daniel Lezcano wrote:
> The governors are defined as module in the code, but the Kconfig options do not
> allow to compile them as module. This is not really a problem but the init
> order is: the cpuidle init functions (framework and driver) and then the
> governors. That leads to some weirdness in the cpuidle framework because the
> function cpuidle_register_device calls cpuidle_enable_device which in turns
> fails at the first attempt because no governor is registered. When the
> governor is registered, the framework calls cpuidle_enable_device again which
> will invoke the __cpuidle_register_device function. Of course, in order to make
> this to work, the return code of cpuidle_enable_device is not checked by the
> caller in cpuidle_register_device.
>
> Instead of having this cyclic call graph and relying on a positive side effect
> of the hackish back and forth call to cpuidle_enable_device, let's fix the
> init order for the governor in order to clean up the cpuidle_enable_device
> function.
>
> Remove the module init code and replaced it with postcore_initcall, so we have:
>
> * cpuidle framework : core_initcall
> * cpuidle governors : postcore_initcall
> * cpuidle drivers : device_initcall
>
> Remove exit module code as it is dead code (governors aren't compiled as
> module).
>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
That's something I'd like to receive much earlier in the cycle (around -rc2).
Honestly, I'm not sure if it's going to make it into 3.11 even if nobody has
any comments.
Thanks,
Rafael
> ---
> drivers/cpuidle/governors/ladder.c | 12 +-----------
> drivers/cpuidle/governors/menu.c | 12 +-----------
> 2 files changed, 2 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/cpuidle/governors/ladder.c b/drivers/cpuidle/governors/ladder.c
> index 9b78405..9f08e8c 100644
> --- a/drivers/cpuidle/governors/ladder.c
> +++ b/drivers/cpuidle/governors/ladder.c
> @@ -192,14 +192,4 @@ static int __init init_ladder(void)
> return cpuidle_register_governor(&ladder_governor);
> }
>
> -/**
> - * exit_ladder - exits the governor
> - */
> -static void __exit exit_ladder(void)
> -{
> - cpuidle_unregister_governor(&ladder_governor);
> -}
> -
> -MODULE_LICENSE("GPL");
> -module_init(init_ladder);
> -module_exit(exit_ladder);
> +postcore_initcall(init_ladder);
> diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c
> index fe343a0..743138c 100644
> --- a/drivers/cpuidle/governors/menu.c
> +++ b/drivers/cpuidle/governors/menu.c
> @@ -540,14 +540,4 @@ static int __init init_menu(void)
> return cpuidle_register_governor(&menu_governor);
> }
>
> -/**
> - * exit_menu - exits the governor
> - */
> -static void __exit exit_menu(void)
> -{
> - cpuidle_unregister_governor(&menu_governor);
> -}
> -
> -MODULE_LICENSE("GPL");
> -module_init(init_menu);
> -module_exit(exit_menu);
> +postcore_initcall(init_menu);
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
next prev parent reply other threads:[~2013-06-12 22:44 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-06-12 13:08 [PATCH 1/8] cpuidle: Remove governor module format Daniel Lezcano
2013-06-12 13:08 ` [PATCH 2/8] cpuidle: Check cpuidle_enable_device succeed Daniel Lezcano
2013-06-12 13:08 ` [PATCH 3/8] cpuidle: Fix indentation and conform to Coding Style Daniel Lezcano
2013-06-12 13:08 ` [PATCH 4/8] cpuidle: Make cpuidle's sysfs directory dynamically allocated Daniel Lezcano
2013-06-12 13:08 ` [PATCH 5/8] cpuidle: Add missing forward declaration Daniel Lezcano
2013-06-12 13:08 ` [PATCH 6/8] cpuidle: Encapsulate code in __cpuidle_unregister_device Daniel Lezcano
2013-06-12 13:08 ` [PATCH 7/8] cpuidle: Add a cpuidle_device init function Daniel Lezcano
2013-06-12 13:08 ` [PATCH 8/8] cpuidle: Check the device is not already registered Daniel Lezcano
2013-06-12 22:53 ` Rafael J. Wysocki [this message]
2013-06-13 11:57 ` [PATCH 1/8] cpuidle: Remove governor module format Daniel Lezcano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1584034.uif9lZguq4@vostro.rjw.lan \
--to=rjw@sisk.pl \
--cc=daniel.lezcano@linaro.org \
--cc=linaro-kernel@lists.linaro.org \
--cc=linux-pm@vger.kernel.org \
--cc=patches@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).