[PATCH V2] cpufreq: Remove CPUFREQ_STICKY flag
Rafael J. Wysocki
rafael at kernel.org
Thu Feb 4 13:24:58 EST 2021
On Tue, Feb 2, 2021 at 5:56 AM Viresh Kumar <viresh.kumar at linaro.org> wrote:
>
> During cpufreq driver's registration, if the ->init() callback for all
> the CPUs fail then there is not much point in keeping the driver around
> as it will only account for more of unnecessary noise, for example
> cpufreq core will try to suspend/resume the driver which never got
> registered properly.
>
> The removal of such a driver is avoided if the driver carries the
> CPUFREQ_STICKY flag. This was added way back [1] in 2004 and perhaps no
> one should ever need it now. A lot of drivers do set this flag, probably
> because they just copied it from other drivers.
>
> This was added earlier for some platforms [2] because their cpufreq
> drivers were getting registered before the CPUs were registered with
> subsys framework. And hence they used to fail.
>
> The same isn't true anymore though. The current code flow in the kernel
> is:
>
> start_kernel()
> -> kernel_init()
> -> kernel_init_freeable()
> -> do_basic_setup()
> -> driver_init()
> -> cpu_dev_init()
> -> subsys_system_register() //For CPUs
>
> -> do_initcalls()
> -> cpufreq_register_driver()
>
> Clearly, the CPUs will always get registered with subsys framework
> before any cpufreq driver can get probed. Remove the flag and update the
> relevant drivers.
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/commit/include/linux/cpufreq.h?id=7cc9f0d9a1ab04cedc60d64fd8dcf7df224a3b4d
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/commit/arch/arm/mach-sa1100/cpu-sa1100.c?id=f59d3bbe35f6268d729f51be82af8325d62f20f5
>
> Signed-off-by: Viresh Kumar <viresh.kumar at linaro.org>
Applied as 5.12 material, thanks!
> ---
> V2: Detailed commit log.
>
> drivers/cpufreq/cpufreq-dt.c | 2 +-
> drivers/cpufreq/cpufreq.c | 3 +--
> drivers/cpufreq/davinci-cpufreq.c | 2 +-
> drivers/cpufreq/loongson1-cpufreq.c | 2 +-
> drivers/cpufreq/mediatek-cpufreq.c | 2 +-
> drivers/cpufreq/omap-cpufreq.c | 2 +-
> drivers/cpufreq/qcom-cpufreq-hw.c | 2 +-
> drivers/cpufreq/s3c24xx-cpufreq.c | 2 +-
> drivers/cpufreq/s5pv210-cpufreq.c | 2 +-
> drivers/cpufreq/sa1100-cpufreq.c | 2 +-
> drivers/cpufreq/sa1110-cpufreq.c | 2 +-
> drivers/cpufreq/scmi-cpufreq.c | 2 +-
> drivers/cpufreq/scpi-cpufreq.c | 2 +-
> drivers/cpufreq/spear-cpufreq.c | 2 +-
> drivers/cpufreq/tegra186-cpufreq.c | 2 +-
> drivers/cpufreq/tegra194-cpufreq.c | 3 +--
> drivers/cpufreq/vexpress-spc-cpufreq.c | 3 +--
> include/linux/cpufreq.h | 17 +++++++----------
> 18 files changed, 24 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c
> index ad4234518ef6..b1e1bdc63b01 100644
> --- a/drivers/cpufreq/cpufreq-dt.c
> +++ b/drivers/cpufreq/cpufreq-dt.c
> @@ -175,7 +175,7 @@ static int cpufreq_exit(struct cpufreq_policy *policy)
> }
>
> static struct cpufreq_driver dt_cpufreq_driver = {
> - .flags = CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK |
> + .flags = CPUFREQ_NEED_INITIAL_FREQ_CHECK |
> CPUFREQ_IS_COOLING_DEV,
> .verify = cpufreq_generic_frequency_table_verify,
> .target_index = set_target,
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index d0a3525ce27f..7d0ae968def7 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -2810,8 +2810,7 @@ int cpufreq_register_driver(struct cpufreq_driver *driver_data)
> if (ret)
> goto err_boost_unreg;
>
> - if (!(cpufreq_driver->flags & CPUFREQ_STICKY) &&
> - list_empty(&cpufreq_policy_list)) {
> + if (unlikely(list_empty(&cpufreq_policy_list))) {
> /* if all ->init() calls failed, unregister */
> ret = -ENODEV;
> pr_debug("%s: No CPU initialized for driver %s\n", __func__,
> diff --git a/drivers/cpufreq/davinci-cpufreq.c b/drivers/cpufreq/davinci-cpufreq.c
> index 91f477a6cbc4..9e97f60f8199 100644
> --- a/drivers/cpufreq/davinci-cpufreq.c
> +++ b/drivers/cpufreq/davinci-cpufreq.c
> @@ -95,7 +95,7 @@ static int davinci_cpu_init(struct cpufreq_policy *policy)
> }
>
> static struct cpufreq_driver davinci_driver = {
> - .flags = CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK,
> + .flags = CPUFREQ_NEED_INITIAL_FREQ_CHECK,
> .verify = cpufreq_generic_frequency_table_verify,
> .target_index = davinci_target,
> .get = cpufreq_generic_get,
> diff --git a/drivers/cpufreq/loongson1-cpufreq.c b/drivers/cpufreq/loongson1-cpufreq.c
> index 86f612593e49..fb72d709db56 100644
> --- a/drivers/cpufreq/loongson1-cpufreq.c
> +++ b/drivers/cpufreq/loongson1-cpufreq.c
> @@ -116,7 +116,7 @@ static int ls1x_cpufreq_exit(struct cpufreq_policy *policy)
>
> static struct cpufreq_driver ls1x_cpufreq_driver = {
> .name = "cpufreq-ls1x",
> - .flags = CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK,
> + .flags = CPUFREQ_NEED_INITIAL_FREQ_CHECK,
> .verify = cpufreq_generic_frequency_table_verify,
> .target_index = ls1x_cpufreq_target,
> .get = cpufreq_generic_get,
> diff --git a/drivers/cpufreq/mediatek-cpufreq.c b/drivers/cpufreq/mediatek-cpufreq.c
> index 022e3e966e71..f2e491b25b07 100644
> --- a/drivers/cpufreq/mediatek-cpufreq.c
> +++ b/drivers/cpufreq/mediatek-cpufreq.c
> @@ -463,7 +463,7 @@ static int mtk_cpufreq_exit(struct cpufreq_policy *policy)
> }
>
> static struct cpufreq_driver mtk_cpufreq_driver = {
> - .flags = CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK |
> + .flags = CPUFREQ_NEED_INITIAL_FREQ_CHECK |
> CPUFREQ_HAVE_GOVERNOR_PER_POLICY |
> CPUFREQ_IS_COOLING_DEV,
> .verify = cpufreq_generic_frequency_table_verify,
> diff --git a/drivers/cpufreq/omap-cpufreq.c b/drivers/cpufreq/omap-cpufreq.c
> index 3694bb030df3..e035ee216b0f 100644
> --- a/drivers/cpufreq/omap-cpufreq.c
> +++ b/drivers/cpufreq/omap-cpufreq.c
> @@ -144,7 +144,7 @@ static int omap_cpu_exit(struct cpufreq_policy *policy)
> }
>
> static struct cpufreq_driver omap_driver = {
> - .flags = CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK,
> + .flags = CPUFREQ_NEED_INITIAL_FREQ_CHECK,
> .verify = cpufreq_generic_frequency_table_verify,
> .target_index = omap_target,
> .get = cpufreq_generic_get,
> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
> index 9ed5341dc515..2a3b4f44488b 100644
> --- a/drivers/cpufreq/qcom-cpufreq-hw.c
> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c
> @@ -374,7 +374,7 @@ static struct freq_attr *qcom_cpufreq_hw_attr[] = {
> };
>
> static struct cpufreq_driver cpufreq_qcom_hw_driver = {
> - .flags = CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK |
> + .flags = CPUFREQ_NEED_INITIAL_FREQ_CHECK |
> CPUFREQ_HAVE_GOVERNOR_PER_POLICY |
> CPUFREQ_IS_COOLING_DEV,
> .verify = cpufreq_generic_frequency_table_verify,
> diff --git a/drivers/cpufreq/s3c24xx-cpufreq.c b/drivers/cpufreq/s3c24xx-cpufreq.c
> index 37efc0dc3f91..7380c32b238e 100644
> --- a/drivers/cpufreq/s3c24xx-cpufreq.c
> +++ b/drivers/cpufreq/s3c24xx-cpufreq.c
> @@ -420,7 +420,7 @@ static int s3c_cpufreq_resume(struct cpufreq_policy *policy)
> #endif
>
> static struct cpufreq_driver s3c24xx_driver = {
> - .flags = CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK,
> + .flags = CPUFREQ_NEED_INITIAL_FREQ_CHECK,
> .target = s3c_cpufreq_target,
> .get = cpufreq_generic_get,
> .init = s3c_cpufreq_init,
> diff --git a/drivers/cpufreq/s5pv210-cpufreq.c b/drivers/cpufreq/s5pv210-cpufreq.c
> index bed496cf8d24..69786e5bbf05 100644
> --- a/drivers/cpufreq/s5pv210-cpufreq.c
> +++ b/drivers/cpufreq/s5pv210-cpufreq.c
> @@ -574,7 +574,7 @@ static int s5pv210_cpufreq_reboot_notifier_event(struct notifier_block *this,
> }
>
> static struct cpufreq_driver s5pv210_driver = {
> - .flags = CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK,
> + .flags = CPUFREQ_NEED_INITIAL_FREQ_CHECK,
> .verify = cpufreq_generic_frequency_table_verify,
> .target_index = s5pv210_target,
> .get = cpufreq_generic_get,
> diff --git a/drivers/cpufreq/sa1100-cpufreq.c b/drivers/cpufreq/sa1100-cpufreq.c
> index 5c075ef6adc0..252b9fc26124 100644
> --- a/drivers/cpufreq/sa1100-cpufreq.c
> +++ b/drivers/cpufreq/sa1100-cpufreq.c
> @@ -186,7 +186,7 @@ static int __init sa1100_cpu_init(struct cpufreq_policy *policy)
> }
>
> static struct cpufreq_driver sa1100_driver __refdata = {
> - .flags = CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK |
> + .flags = CPUFREQ_NEED_INITIAL_FREQ_CHECK |
> CPUFREQ_NO_AUTO_DYNAMIC_SWITCHING,
> .verify = cpufreq_generic_frequency_table_verify,
> .target_index = sa1100_target,
> diff --git a/drivers/cpufreq/sa1110-cpufreq.c b/drivers/cpufreq/sa1110-cpufreq.c
> index d9d04d935b3a..1a83c8678a63 100644
> --- a/drivers/cpufreq/sa1110-cpufreq.c
> +++ b/drivers/cpufreq/sa1110-cpufreq.c
> @@ -310,7 +310,7 @@ static int __init sa1110_cpu_init(struct cpufreq_policy *policy)
> /* sa1110_driver needs __refdata because it must remain after init registers
> * it with cpufreq_register_driver() */
> static struct cpufreq_driver sa1110_driver __refdata = {
> - .flags = CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK |
> + .flags = CPUFREQ_NEED_INITIAL_FREQ_CHECK |
> CPUFREQ_NO_AUTO_DYNAMIC_SWITCHING,
> .verify = cpufreq_generic_frequency_table_verify,
> .target_index = sa1110_target,
> diff --git a/drivers/cpufreq/scmi-cpufreq.c b/drivers/cpufreq/scmi-cpufreq.c
> index 491a0a24fb1e..5bd03b59887f 100644
> --- a/drivers/cpufreq/scmi-cpufreq.c
> +++ b/drivers/cpufreq/scmi-cpufreq.c
> @@ -217,7 +217,7 @@ static int scmi_cpufreq_exit(struct cpufreq_policy *policy)
>
> static struct cpufreq_driver scmi_cpufreq_driver = {
> .name = "scmi",
> - .flags = CPUFREQ_STICKY | CPUFREQ_HAVE_GOVERNOR_PER_POLICY |
> + .flags = CPUFREQ_HAVE_GOVERNOR_PER_POLICY |
> CPUFREQ_NEED_INITIAL_FREQ_CHECK |
> CPUFREQ_IS_COOLING_DEV,
> .verify = cpufreq_generic_frequency_table_verify,
> diff --git a/drivers/cpufreq/scpi-cpufreq.c b/drivers/cpufreq/scpi-cpufreq.c
> index e5140ad63db8..d6a698a1b5d1 100644
> --- a/drivers/cpufreq/scpi-cpufreq.c
> +++ b/drivers/cpufreq/scpi-cpufreq.c
> @@ -191,7 +191,7 @@ static int scpi_cpufreq_exit(struct cpufreq_policy *policy)
>
> static struct cpufreq_driver scpi_cpufreq_driver = {
> .name = "scpi-cpufreq",
> - .flags = CPUFREQ_STICKY | CPUFREQ_HAVE_GOVERNOR_PER_POLICY |
> + .flags = CPUFREQ_HAVE_GOVERNOR_PER_POLICY |
> CPUFREQ_NEED_INITIAL_FREQ_CHECK |
> CPUFREQ_IS_COOLING_DEV,
> .verify = cpufreq_generic_frequency_table_verify,
> diff --git a/drivers/cpufreq/spear-cpufreq.c b/drivers/cpufreq/spear-cpufreq.c
> index 73bd8dc47074..7d0d62a06bf3 100644
> --- a/drivers/cpufreq/spear-cpufreq.c
> +++ b/drivers/cpufreq/spear-cpufreq.c
> @@ -160,7 +160,7 @@ static int spear_cpufreq_init(struct cpufreq_policy *policy)
>
> static struct cpufreq_driver spear_cpufreq_driver = {
> .name = "cpufreq-spear",
> - .flags = CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK,
> + .flags = CPUFREQ_NEED_INITIAL_FREQ_CHECK,
> .verify = cpufreq_generic_frequency_table_verify,
> .target_index = spear_cpufreq_target,
> .get = cpufreq_generic_get,
> diff --git a/drivers/cpufreq/tegra186-cpufreq.c b/drivers/cpufreq/tegra186-cpufreq.c
> index e566ea298b59..5d1943e787b0 100644
> --- a/drivers/cpufreq/tegra186-cpufreq.c
> +++ b/drivers/cpufreq/tegra186-cpufreq.c
> @@ -117,7 +117,7 @@ static unsigned int tegra186_cpufreq_get(unsigned int cpu)
>
> static struct cpufreq_driver tegra186_cpufreq_driver = {
> .name = "tegra186",
> - .flags = CPUFREQ_STICKY | CPUFREQ_HAVE_GOVERNOR_PER_POLICY |
> + .flags = CPUFREQ_HAVE_GOVERNOR_PER_POLICY |
> CPUFREQ_NEED_INITIAL_FREQ_CHECK,
> .get = tegra186_cpufreq_get,
> .verify = cpufreq_generic_frequency_table_verify,
> diff --git a/drivers/cpufreq/tegra194-cpufreq.c b/drivers/cpufreq/tegra194-cpufreq.c
> index 6a67f36f3b80..a9620e4489ae 100644
> --- a/drivers/cpufreq/tegra194-cpufreq.c
> +++ b/drivers/cpufreq/tegra194-cpufreq.c
> @@ -272,8 +272,7 @@ static int tegra194_cpufreq_set_target(struct cpufreq_policy *policy,
>
> static struct cpufreq_driver tegra194_cpufreq_driver = {
> .name = "tegra194",
> - .flags = CPUFREQ_STICKY | CPUFREQ_CONST_LOOPS |
> - CPUFREQ_NEED_INITIAL_FREQ_CHECK,
> + .flags = CPUFREQ_CONST_LOOPS | CPUFREQ_NEED_INITIAL_FREQ_CHECK,
> .verify = cpufreq_generic_frequency_table_verify,
> .target_index = tegra194_cpufreq_set_target,
> .get = tegra194_get_speed,
> diff --git a/drivers/cpufreq/vexpress-spc-cpufreq.c b/drivers/cpufreq/vexpress-spc-cpufreq.c
> index f711d8eaea6a..51dfa9ae6cf5 100644
> --- a/drivers/cpufreq/vexpress-spc-cpufreq.c
> +++ b/drivers/cpufreq/vexpress-spc-cpufreq.c
> @@ -486,8 +486,7 @@ static void ve_spc_cpufreq_ready(struct cpufreq_policy *policy)
>
> static struct cpufreq_driver ve_spc_cpufreq_driver = {
> .name = "vexpress-spc",
> - .flags = CPUFREQ_STICKY |
> - CPUFREQ_HAVE_GOVERNOR_PER_POLICY |
> + .flags = CPUFREQ_HAVE_GOVERNOR_PER_POLICY |
> CPUFREQ_NEED_INITIAL_FREQ_CHECK,
> .verify = cpufreq_generic_frequency_table_verify,
> .target_index = ve_spc_cpufreq_set_target,
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index 9c8b7437b6cd..c8e40e91fe9b 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -387,8 +387,13 @@ struct cpufreq_driver {
>
> /* flags */
>
> -/* driver isn't removed even if all ->init() calls failed */
> -#define CPUFREQ_STICKY BIT(0)
> +/*
> + * Set by drivers that need to update internale upper and lower boundaries along
> + * with the target frequency and so the core and governors should also invoke
> + * the diver if the target frequency does not change, but the policy min or max
> + * may have changed.
> + */
> +#define CPUFREQ_NEED_UPDATE_LIMITS BIT(0)
>
> /* loops_per_jiffy or other kernel "constants" aren't affected by frequency transitions */
> #define CPUFREQ_CONST_LOOPS BIT(1)
> @@ -432,14 +437,6 @@ struct cpufreq_driver {
> */
> #define CPUFREQ_IS_COOLING_DEV BIT(7)
>
> -/*
> - * Set by drivers that need to update internale upper and lower boundaries along
> - * with the target frequency and so the core and governors should also invoke
> - * the diver if the target frequency does not change, but the policy min or max
> - * may have changed.
> - */
> -#define CPUFREQ_NEED_UPDATE_LIMITS BIT(8)
> -
> int cpufreq_register_driver(struct cpufreq_driver *driver_data);
> int cpufreq_unregister_driver(struct cpufreq_driver *driver_data);
>
> --
> 2.25.0.rc1.19.g042ed3e048af
>
More information about the linux-arm-kernel
mailing list