On Tue, Feb 2, 2021 at 5:56 AM Viresh Kumar <viresh.kumar@xxxxxxxxxx> 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@xxxxxxxxxx> 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 >
