On some systems we can't really say what frequency we're running at the moment and so for these we shouldn't check if we are running at a frequency present in frequency table. For now mark all x86 drivers with this flag: CPUFREQ_SKIP_INITIAL_FREQ_CHECK.
This patch also defines this macro. It will be used in cpufreq.c in the next patch.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- V3->V4: - Reversed the order of patches - Moved definition of CPUFREQ_SKIP_INITIAL_FREQ_CHECK to patch 1/2 instead of 2/2.
drivers/cpufreq/acpi-cpufreq.c | 1 + drivers/cpufreq/cpufreq-nforce2.c | 1 + drivers/cpufreq/e_powersaver.c | 1 + drivers/cpufreq/elanfreq.c | 1 + drivers/cpufreq/gx-suspmod.c | 1 + drivers/cpufreq/ia64-acpi-cpufreq.c | 1 + drivers/cpufreq/intel_pstate.c | 2 +- drivers/cpufreq/longhaul.c | 1 + drivers/cpufreq/longrun.c | 2 +- drivers/cpufreq/p4-clockmod.c | 1 + drivers/cpufreq/pcc-cpufreq.c | 2 +- drivers/cpufreq/powernow-k6.c | 1 + drivers/cpufreq/powernow-k7.c | 1 + drivers/cpufreq/powernow-k8.c | 3 ++- drivers/cpufreq/sc520_freq.c | 1 + drivers/cpufreq/speedstep-centrino.c | 1 + drivers/cpufreq/speedstep-ich.c | 1 + drivers/cpufreq/speedstep-smi.c | 1 + include/linux/cpufreq.h | 9 +++++++++ 19 files changed, 28 insertions(+), 4 deletions(-)
diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c index caf41eb..b8e4f79 100644 --- a/drivers/cpufreq/acpi-cpufreq.c +++ b/drivers/cpufreq/acpi-cpufreq.c @@ -903,6 +903,7 @@ static struct freq_attr *acpi_cpufreq_attr[] = { };
static struct cpufreq_driver acpi_cpufreq_driver = { + .flags = CPUFREQ_SKIP_INITIAL_FREQ_CHECK, .verify = cpufreq_generic_frequency_table_verify, .target_index = acpi_cpufreq_target, .bios_limit = acpi_processor_get_bios_limit, diff --git a/drivers/cpufreq/cpufreq-nforce2.c b/drivers/cpufreq/cpufreq-nforce2.c index a05b876..b3222f6 100644 --- a/drivers/cpufreq/cpufreq-nforce2.c +++ b/drivers/cpufreq/cpufreq-nforce2.c @@ -370,6 +370,7 @@ static int nforce2_cpu_exit(struct cpufreq_policy *policy) }
static struct cpufreq_driver nforce2_driver = { + .flags = CPUFREQ_SKIP_INITIAL_FREQ_CHECK, .name = "nforce2", .verify = nforce2_verify, .target = nforce2_target, diff --git a/drivers/cpufreq/e_powersaver.c b/drivers/cpufreq/e_powersaver.c index 9012b8b..0826bda 100644 --- a/drivers/cpufreq/e_powersaver.c +++ b/drivers/cpufreq/e_powersaver.c @@ -389,6 +389,7 @@ static int eps_cpu_exit(struct cpufreq_policy *policy) }
static struct cpufreq_driver eps_driver = { + .flags = CPUFREQ_SKIP_INITIAL_FREQ_CHECK, .verify = cpufreq_generic_frequency_table_verify, .target_index = eps_target, .init = eps_cpu_init, diff --git a/drivers/cpufreq/elanfreq.c b/drivers/cpufreq/elanfreq.c index de08acf..426d14d 100644 --- a/drivers/cpufreq/elanfreq.c +++ b/drivers/cpufreq/elanfreq.c @@ -194,6 +194,7 @@ __setup("elanfreq=", elanfreq_setup);
static struct cpufreq_driver elanfreq_driver = { + .flags = CPUFREQ_SKIP_INITIAL_FREQ_CHECK, .get = elanfreq_get_cpu_frequency, .verify = cpufreq_generic_frequency_table_verify, .target_index = elanfreq_target, diff --git a/drivers/cpufreq/gx-suspmod.c b/drivers/cpufreq/gx-suspmod.c index d83e826..7bfad48 100644 --- a/drivers/cpufreq/gx-suspmod.c +++ b/drivers/cpufreq/gx-suspmod.c @@ -438,6 +438,7 @@ static int cpufreq_gx_cpu_init(struct cpufreq_policy *policy) * MediaGX/Geode GX initialize cpufreq driver */ static struct cpufreq_driver gx_suspmod_driver = { + .flags = CPUFREQ_SKIP_INITIAL_FREQ_CHECK, .get = gx_get_cpuspeed, .verify = cpufreq_gx_verify, .target = cpufreq_gx_target, diff --git a/drivers/cpufreq/ia64-acpi-cpufreq.c b/drivers/cpufreq/ia64-acpi-cpufreq.c index 53c6ac6..907bf9a 100644 --- a/drivers/cpufreq/ia64-acpi-cpufreq.c +++ b/drivers/cpufreq/ia64-acpi-cpufreq.c @@ -344,6 +344,7 @@ acpi_cpufreq_cpu_exit (
static struct cpufreq_driver acpi_cpufreq_driver = { + .flags = CPUFREQ_SKIP_INITIAL_FREQ_CHECK, .verify = cpufreq_generic_frequency_table_verify, .target_index = acpi_cpufreq_target, .get = acpi_cpufreq_get, diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c index 5f1cbae..6c8f5d3 100644 --- a/drivers/cpufreq/intel_pstate.c +++ b/drivers/cpufreq/intel_pstate.c @@ -725,7 +725,7 @@ static int intel_pstate_cpu_init(struct cpufreq_policy *policy) }
static struct cpufreq_driver intel_pstate_driver = { - .flags = CPUFREQ_CONST_LOOPS, + .flags = CPUFREQ_CONST_LOOPS | CPUFREQ_SKIP_INITIAL_FREQ_CHECK, .verify = intel_pstate_verify_policy, .setpolicy = intel_pstate_set_policy, .get = intel_pstate_get, diff --git a/drivers/cpufreq/longhaul.c b/drivers/cpufreq/longhaul.c index 45bafdd..16c0a37 100644 --- a/drivers/cpufreq/longhaul.c +++ b/drivers/cpufreq/longhaul.c @@ -909,6 +909,7 @@ static int longhaul_cpu_init(struct cpufreq_policy *policy) }
static struct cpufreq_driver longhaul_driver = { + .flags = CPUFREQ_SKIP_INITIAL_FREQ_CHECK, .verify = cpufreq_generic_frequency_table_verify, .target_index = longhaul_target, .get = longhaul_get, diff --git a/drivers/cpufreq/longrun.c b/drivers/cpufreq/longrun.c index 074971b..b67d341 100644 --- a/drivers/cpufreq/longrun.c +++ b/drivers/cpufreq/longrun.c @@ -278,7 +278,7 @@ static int longrun_cpu_init(struct cpufreq_policy *policy)
static struct cpufreq_driver longrun_driver = { - .flags = CPUFREQ_CONST_LOOPS, + .flags = CPUFREQ_CONST_LOOPS | CPUFREQ_SKIP_INITIAL_FREQ_CHECK, .verify = longrun_verify_policy, .setpolicy = longrun_set_policy, .get = longrun_get, diff --git a/drivers/cpufreq/p4-clockmod.c b/drivers/cpufreq/p4-clockmod.c index 3d1cba9..92b7e2b 100644 --- a/drivers/cpufreq/p4-clockmod.c +++ b/drivers/cpufreq/p4-clockmod.c @@ -234,6 +234,7 @@ static unsigned int cpufreq_p4_get(unsigned int cpu) }
static struct cpufreq_driver p4clockmod_driver = { + .flags = CPUFREQ_SKIP_INITIAL_FREQ_CHECK, .verify = cpufreq_generic_frequency_table_verify, .target_index = cpufreq_p4_target, .init = cpufreq_p4_cpu_init, diff --git a/drivers/cpufreq/pcc-cpufreq.c b/drivers/cpufreq/pcc-cpufreq.c index e2b4f40..eafe4ca 100644 --- a/drivers/cpufreq/pcc-cpufreq.c +++ b/drivers/cpufreq/pcc-cpufreq.c @@ -571,7 +571,7 @@ static int pcc_cpufreq_cpu_exit(struct cpufreq_policy *policy) }
static struct cpufreq_driver pcc_cpufreq_driver = { - .flags = CPUFREQ_CONST_LOOPS, + .flags = CPUFREQ_CONST_LOOPS | CPUFREQ_SKIP_INITIAL_FREQ_CHECK, .get = pcc_get_freq, .verify = pcc_cpufreq_verify, .target = pcc_cpufreq_target, diff --git a/drivers/cpufreq/powernow-k6.c b/drivers/cpufreq/powernow-k6.c index 643e795..fe60021 100644 --- a/drivers/cpufreq/powernow-k6.c +++ b/drivers/cpufreq/powernow-k6.c @@ -150,6 +150,7 @@ static unsigned int powernow_k6_get(unsigned int cpu) }
static struct cpufreq_driver powernow_k6_driver = { + .flags = CPUFREQ_SKIP_INITIAL_FREQ_CHECK, .verify = cpufreq_generic_frequency_table_verify, .target_index = powernow_k6_target, .init = powernow_k6_cpu_init, diff --git a/drivers/cpufreq/powernow-k7.c b/drivers/cpufreq/powernow-k7.c index 946708a..4e67abc 100644 --- a/drivers/cpufreq/powernow-k7.c +++ b/drivers/cpufreq/powernow-k7.c @@ -679,6 +679,7 @@ static int powernow_cpu_exit(struct cpufreq_policy *policy) }
static struct cpufreq_driver powernow_driver = { + .flags = CPUFREQ_SKIP_INITIAL_FREQ_CHECK, .verify = cpufreq_generic_frequency_table_verify, .target_index = powernow_target, .get = powernow_get, diff --git a/drivers/cpufreq/powernow-k8.c b/drivers/cpufreq/powernow-k8.c index 0023c7d..ffe89ba 100644 --- a/drivers/cpufreq/powernow-k8.c +++ b/drivers/cpufreq/powernow-k8.c @@ -1204,7 +1204,8 @@ out: }
static struct cpufreq_driver cpufreq_amd64_driver = { - .flags = CPUFREQ_ASYNC_NOTIFICATION, + .flags = CPUFREQ_ASYNC_NOTIFICATION | + CPUFREQ_SKIP_INITIAL_FREQ_CHECK, .verify = cpufreq_generic_frequency_table_verify, .target_index = powernowk8_target, .bios_limit = acpi_processor_get_bios_limit, diff --git a/drivers/cpufreq/sc520_freq.c b/drivers/cpufreq/sc520_freq.c index 6adb354..d6a1cf6 100644 --- a/drivers/cpufreq/sc520_freq.c +++ b/drivers/cpufreq/sc520_freq.c @@ -89,6 +89,7 @@ static int sc520_freq_cpu_init(struct cpufreq_policy *policy)
static struct cpufreq_driver sc520_freq_driver = { + .flags = CPUFREQ_SKIP_INITIAL_FREQ_CHECK, .get = sc520_freq_get_cpu_frequency, .verify = cpufreq_generic_frequency_table_verify, .target_index = sc520_freq_target, diff --git a/drivers/cpufreq/speedstep-centrino.c b/drivers/cpufreq/speedstep-centrino.c index 4e1daca..887fd43 100644 --- a/drivers/cpufreq/speedstep-centrino.c +++ b/drivers/cpufreq/speedstep-centrino.c @@ -505,6 +505,7 @@ out: }
static struct cpufreq_driver centrino_driver = { + .flags = CPUFREQ_SKIP_INITIAL_FREQ_CHECK, .name = "centrino", /* should be speedstep-centrino, but there's a 16 char limit */ .init = centrino_cpu_init, diff --git a/drivers/cpufreq/speedstep-ich.c b/drivers/cpufreq/speedstep-ich.c index 7639b2b..67596e7 100644 --- a/drivers/cpufreq/speedstep-ich.c +++ b/drivers/cpufreq/speedstep-ich.c @@ -307,6 +307,7 @@ static int speedstep_cpu_init(struct cpufreq_policy *policy)
static struct cpufreq_driver speedstep_driver = { + .flags = CPUFREQ_SKIP_INITIAL_FREQ_CHECK, .name = "speedstep-ich", .verify = cpufreq_generic_frequency_table_verify, .target_index = speedstep_target, diff --git a/drivers/cpufreq/speedstep-smi.c b/drivers/cpufreq/speedstep-smi.c index 0f5326d..4f23289 100644 --- a/drivers/cpufreq/speedstep-smi.c +++ b/drivers/cpufreq/speedstep-smi.c @@ -308,6 +308,7 @@ static int speedstep_resume(struct cpufreq_policy *policy) }
static struct cpufreq_driver speedstep_driver = { + .flags = CPUFREQ_SKIP_INITIAL_FREQ_CHECK, .name = "speedstep-smi", .verify = cpufreq_generic_frequency_table_verify, .target_index = speedstep_target, diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h index ee5fe9d..13b8802 100644 --- a/include/linux/cpufreq.h +++ b/include/linux/cpufreq.h @@ -252,6 +252,15 @@ struct cpufreq_driver { */ #define CPUFREQ_ASYNC_NOTIFICATION (1 << 4)
+/* + * Set by drivers which don't want cpufreq core to check if CPU is running at a + * frequency present in freq-table exposed by the driver. For other drivers if + * CPU is found running at an out of table freq, we will try to set it to a freq + * from the table. And if that fails, we will stop further boot process by + * issuing a BUG_ON(). + */ +#define CPUFREQ_SKIP_INITIAL_FREQ_CHECK (1 << 5) + int cpufreq_register_driver(struct cpufreq_driver *driver_data); int cpufreq_unregister_driver(struct cpufreq_driver *driver_data);
Sometimes boot loaders set CPU frequency to a value outside of frequency table present with cpufreq core. In such cases CPU might be unstable if it has to run on that frequency for long duration of time and so its better to set it to a frequency which is specified in freq-table. This also makes cpufreq stats inconsistent as cpufreq-stats would fail to register because current frequency of CPU isn't found in freq-table.
Because we don't want this change to effect boot process badly, we go for the next freq which is >= policy->cur ('cur' must be set by now, otherwise we will end up setting freq to lowest of the table as 'cur' is initialized to zero).
In case current frequency doesn't match any frequency from freq-table, we throw warnings to user, so that user can get this fixed in their bootloaders or freq-tables.
Reported-by: Carlos Hernandez ceh@ti.com Reported-and-tested-by: Nishanth Menon nm@ti.com Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/cpufreq/cpufreq.c | 40 ++++++++++++++++++++++++++++++++++++++++ drivers/cpufreq/freq_table.c | 22 ++++++++++++++++++++++ include/linux/cpufreq.h | 2 ++ 3 files changed, 64 insertions(+)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 606224a..f13320a 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1042,6 +1042,46 @@ static int __cpufreq_add_dev(struct device *dev, struct subsys_interface *sif, } }
+ /* + * Sometimes boot loaders set CPU frequency to a value outside of + * frequency table present with cpufreq core. In such cases CPU might be + * unstable if it has to run on that frequency for long duration of time + * and so its better to set it to a frequency which is specified in + * freq-table. This also makes cpufreq stats inconsistent as + * cpufreq-stats would fail to register because current frequency of CPU + * isn't found in freq-table. + * + * Because we don't want this change to effect boot process badly, we go + * for the next freq which is >= policy->cur ('cur' must be set by now, + * otherwise we will end up setting freq to lowest of the table as 'cur' + * is initialized to zero). + * + * We are passing target-freq as "policy->cur - 1" otherwise + * __cpufreq_driver_target() would simply fail, as policy->cur will be + * equal to target-freq. + */ + if (!(cpufreq_driver->flags & CPUFREQ_SKIP_INITIAL_FREQ_CHECK) + && has_target()) { + /* Are we running at unknown frequency ? */ + ret = cpufreq_frequency_table_get_index(policy, policy->cur); + if (ret == -EINVAL) { + /* Warn user and fix it */ + pr_warn("%s: CPU%d: Running at unlisted freq: %u KHz\n", + __func__, policy->cpu, policy->cur); + ret = __cpufreq_driver_target(policy, policy->cur - 1, + CPUFREQ_RELATION_L); + + /* + * Reaching here after boot in a few seconds may not + * mean that system will remain stable at "unknown" + * frequency for longer duration. Hence, a BUG_ON(). + */ + BUG_ON(ret); + pr_warn("%s: CPU%d: Unlisted initial frequency changed to: %u KHz\n", + __func__, policy->cpu, policy->cur); + } + } + /* related cpus should atleast have policy->cpus */ cpumask_or(policy->related_cpus, policy->related_cpus, policy->cpus);
diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c index 3458d27..a8ac042 100644 --- a/drivers/cpufreq/freq_table.c +++ b/drivers/cpufreq/freq_table.c @@ -178,7 +178,29 @@ int cpufreq_frequency_table_target(struct cpufreq_policy *policy, } EXPORT_SYMBOL_GPL(cpufreq_frequency_table_target);
+int cpufreq_frequency_table_get_index(struct cpufreq_policy *policy, + unsigned int freq) +{ + struct cpufreq_frequency_table *table; + int i; + + table = cpufreq_frequency_get_table(policy->cpu); + if (unlikely(!table)) { + pr_debug("%s: Unable to find frequency table\n", __func__); + return -ENOENT; + } + + for (i = 0; table[i].frequency != CPUFREQ_TABLE_END; i++) { + if (table[i].frequency == freq) + return i; + } + + return -EINVAL; +} +EXPORT_SYMBOL_GPL(cpufreq_frequency_table_get_index); + static DEFINE_PER_CPU(struct cpufreq_frequency_table *, cpufreq_show_table); + /** * show_available_freqs - show available frequencies for the specified CPU */ diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h index 13b8802..03174ab 100644 --- a/include/linux/cpufreq.h +++ b/include/linux/cpufreq.h @@ -456,6 +456,8 @@ int cpufreq_frequency_table_target(struct cpufreq_policy *policy, unsigned int target_freq, unsigned int relation, unsigned int *index); +int cpufreq_frequency_table_get_index(struct cpufreq_policy *policy, + unsigned int freq);
void cpufreq_frequency_table_update_policy_cpu(struct cpufreq_policy *policy); ssize_t cpufreq_show_cpus(const struct cpumask *mask, char *buf);
Sorry for late response to these threads. Holiday weekend in the US.
On 11/29/2013 06:14 AM, Viresh Kumar wrote:
On some systems we can't really say what frequency we're running at the moment and so for these we shouldn't check if we are running at a frequency present in frequency table. For now mark all x86 drivers with this flag: CPUFREQ_SKIP_INITIAL_FREQ_CHECK.
This patch also defines this macro. It will be used in cpufreq.c in the next patch.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org
V3->V4:
Reversed the order of patches
Moved definition of CPUFREQ_SKIP_INITIAL_FREQ_CHECK to patch 1/2 instead of 2/2.
drivers/cpufreq/acpi-cpufreq.c | 1 + drivers/cpufreq/cpufreq-nforce2.c | 1 + drivers/cpufreq/e_powersaver.c | 1 + drivers/cpufreq/elanfreq.c | 1 + drivers/cpufreq/gx-suspmod.c | 1 + drivers/cpufreq/ia64-acpi-cpufreq.c | 1 + drivers/cpufreq/intel_pstate.c | 2 +-
NAK for intel_pstate. The code that will use this flag also uses has_target() which is false for intel_pstate.
I think the logic for this patch is inverted. Why are we adding a flag to drivers where the bug cannot exist? I would be happier if the flag was CPUFREQ_NEED_INITIAL_FREQ_WORKAROUND to mark the platforms where the bug may exist let those platforms cary the flag for the next 10+ years.
I still believe that this bug should be dealt with in platform specific code since this is working around a bootloader bug. Are there platforms in the wild that even need this code or is this to support early platform bringup?
drivers/cpufreq/longhaul.c | 1 + drivers/cpufreq/longrun.c | 2 +- drivers/cpufreq/p4-clockmod.c | 1 + drivers/cpufreq/pcc-cpufreq.c | 2 +- drivers/cpufreq/powernow-k6.c | 1 + drivers/cpufreq/powernow-k7.c | 1 + drivers/cpufreq/powernow-k8.c | 3 ++- drivers/cpufreq/sc520_freq.c | 1 + drivers/cpufreq/speedstep-centrino.c | 1 + drivers/cpufreq/speedstep-ich.c | 1 + drivers/cpufreq/speedstep-smi.c | 1 + include/linux/cpufreq.h | 9 +++++++++ 19 files changed, 28 insertions(+), 4 deletions(-)
diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c index caf41eb..b8e4f79 100644 --- a/drivers/cpufreq/acpi-cpufreq.c +++ b/drivers/cpufreq/acpi-cpufreq.c @@ -903,6 +903,7 @@ static struct freq_attr *acpi_cpufreq_attr[] = { };
static struct cpufreq_driver acpi_cpufreq_driver = {
- .flags = CPUFREQ_SKIP_INITIAL_FREQ_CHECK, .verify = cpufreq_generic_frequency_table_verify, .target_index = acpi_cpufreq_target, .bios_limit = acpi_processor_get_bios_limit,
diff --git a/drivers/cpufreq/cpufreq-nforce2.c b/drivers/cpufreq/cpufreq-nforce2.c index a05b876..b3222f6 100644 --- a/drivers/cpufreq/cpufreq-nforce2.c +++ b/drivers/cpufreq/cpufreq-nforce2.c @@ -370,6 +370,7 @@ static int nforce2_cpu_exit(struct cpufreq_policy *policy) }
static struct cpufreq_driver nforce2_driver = {
- .flags = CPUFREQ_SKIP_INITIAL_FREQ_CHECK, .name = "nforce2", .verify = nforce2_verify, .target = nforce2_target,
diff --git a/drivers/cpufreq/e_powersaver.c b/drivers/cpufreq/e_powersaver.c index 9012b8b..0826bda 100644 --- a/drivers/cpufreq/e_powersaver.c +++ b/drivers/cpufreq/e_powersaver.c @@ -389,6 +389,7 @@ static int eps_cpu_exit(struct cpufreq_policy *policy) }
static struct cpufreq_driver eps_driver = {
- .flags = CPUFREQ_SKIP_INITIAL_FREQ_CHECK, .verify = cpufreq_generic_frequency_table_verify, .target_index = eps_target, .init = eps_cpu_init,
diff --git a/drivers/cpufreq/elanfreq.c b/drivers/cpufreq/elanfreq.c index de08acf..426d14d 100644 --- a/drivers/cpufreq/elanfreq.c +++ b/drivers/cpufreq/elanfreq.c @@ -194,6 +194,7 @@ __setup("elanfreq=", elanfreq_setup);
static struct cpufreq_driver elanfreq_driver = {
- .flags = CPUFREQ_SKIP_INITIAL_FREQ_CHECK, .get = elanfreq_get_cpu_frequency, .verify = cpufreq_generic_frequency_table_verify, .target_index = elanfreq_target,
diff --git a/drivers/cpufreq/gx-suspmod.c b/drivers/cpufreq/gx-suspmod.c index d83e826..7bfad48 100644 --- a/drivers/cpufreq/gx-suspmod.c +++ b/drivers/cpufreq/gx-suspmod.c @@ -438,6 +438,7 @@ static int cpufreq_gx_cpu_init(struct cpufreq_policy *policy)
- MediaGX/Geode GX initialize cpufreq driver
*/ static struct cpufreq_driver gx_suspmod_driver = {
- .flags = CPUFREQ_SKIP_INITIAL_FREQ_CHECK, .get = gx_get_cpuspeed, .verify = cpufreq_gx_verify, .target = cpufreq_gx_target,
diff --git a/drivers/cpufreq/ia64-acpi-cpufreq.c b/drivers/cpufreq/ia64-acpi-cpufreq.c index 53c6ac6..907bf9a 100644 --- a/drivers/cpufreq/ia64-acpi-cpufreq.c +++ b/drivers/cpufreq/ia64-acpi-cpufreq.c @@ -344,6 +344,7 @@ acpi_cpufreq_cpu_exit (
static struct cpufreq_driver acpi_cpufreq_driver = {
- .flags = CPUFREQ_SKIP_INITIAL_FREQ_CHECK, .verify = cpufreq_generic_frequency_table_verify, .target_index = acpi_cpufreq_target, .get = acpi_cpufreq_get,
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c index 5f1cbae..6c8f5d3 100644 --- a/drivers/cpufreq/intel_pstate.c +++ b/drivers/cpufreq/intel_pstate.c @@ -725,7 +725,7 @@ static int intel_pstate_cpu_init(struct cpufreq_policy *policy) }
static struct cpufreq_driver intel_pstate_driver = {
- .flags = CPUFREQ_CONST_LOOPS,
- .flags = CPUFREQ_CONST_LOOPS | CPUFREQ_SKIP_INITIAL_FREQ_CHECK, .verify = intel_pstate_verify_policy, .setpolicy = intel_pstate_set_policy, .get = intel_pstate_get,
diff --git a/drivers/cpufreq/longhaul.c b/drivers/cpufreq/longhaul.c index 45bafdd..16c0a37 100644 --- a/drivers/cpufreq/longhaul.c +++ b/drivers/cpufreq/longhaul.c @@ -909,6 +909,7 @@ static int longhaul_cpu_init(struct cpufreq_policy *policy) }
static struct cpufreq_driver longhaul_driver = {
- .flags = CPUFREQ_SKIP_INITIAL_FREQ_CHECK, .verify = cpufreq_generic_frequency_table_verify, .target_index = longhaul_target, .get = longhaul_get,
diff --git a/drivers/cpufreq/longrun.c b/drivers/cpufreq/longrun.c index 074971b..b67d341 100644 --- a/drivers/cpufreq/longrun.c +++ b/drivers/cpufreq/longrun.c @@ -278,7 +278,7 @@ static int longrun_cpu_init(struct cpufreq_policy *policy)
static struct cpufreq_driver longrun_driver = {
- .flags = CPUFREQ_CONST_LOOPS,
- .flags = CPUFREQ_CONST_LOOPS | CPUFREQ_SKIP_INITIAL_FREQ_CHECK, .verify = longrun_verify_policy, .setpolicy = longrun_set_policy, .get = longrun_get,
diff --git a/drivers/cpufreq/p4-clockmod.c b/drivers/cpufreq/p4-clockmod.c index 3d1cba9..92b7e2b 100644 --- a/drivers/cpufreq/p4-clockmod.c +++ b/drivers/cpufreq/p4-clockmod.c @@ -234,6 +234,7 @@ static unsigned int cpufreq_p4_get(unsigned int cpu) }
static struct cpufreq_driver p4clockmod_driver = {
- .flags = CPUFREQ_SKIP_INITIAL_FREQ_CHECK, .verify = cpufreq_generic_frequency_table_verify, .target_index = cpufreq_p4_target, .init = cpufreq_p4_cpu_init,
diff --git a/drivers/cpufreq/pcc-cpufreq.c b/drivers/cpufreq/pcc-cpufreq.c index e2b4f40..eafe4ca 100644 --- a/drivers/cpufreq/pcc-cpufreq.c +++ b/drivers/cpufreq/pcc-cpufreq.c @@ -571,7 +571,7 @@ static int pcc_cpufreq_cpu_exit(struct cpufreq_policy *policy) }
static struct cpufreq_driver pcc_cpufreq_driver = {
- .flags = CPUFREQ_CONST_LOOPS,
- .flags = CPUFREQ_CONST_LOOPS | CPUFREQ_SKIP_INITIAL_FREQ_CHECK, .get = pcc_get_freq, .verify = pcc_cpufreq_verify, .target = pcc_cpufreq_target,
diff --git a/drivers/cpufreq/powernow-k6.c b/drivers/cpufreq/powernow-k6.c index 643e795..fe60021 100644 --- a/drivers/cpufreq/powernow-k6.c +++ b/drivers/cpufreq/powernow-k6.c @@ -150,6 +150,7 @@ static unsigned int powernow_k6_get(unsigned int cpu) }
static struct cpufreq_driver powernow_k6_driver = {
- .flags = CPUFREQ_SKIP_INITIAL_FREQ_CHECK, .verify = cpufreq_generic_frequency_table_verify, .target_index = powernow_k6_target, .init = powernow_k6_cpu_init,
diff --git a/drivers/cpufreq/powernow-k7.c b/drivers/cpufreq/powernow-k7.c index 946708a..4e67abc 100644 --- a/drivers/cpufreq/powernow-k7.c +++ b/drivers/cpufreq/powernow-k7.c @@ -679,6 +679,7 @@ static int powernow_cpu_exit(struct cpufreq_policy *policy) }
static struct cpufreq_driver powernow_driver = {
- .flags = CPUFREQ_SKIP_INITIAL_FREQ_CHECK, .verify = cpufreq_generic_frequency_table_verify, .target_index = powernow_target, .get = powernow_get,
diff --git a/drivers/cpufreq/powernow-k8.c b/drivers/cpufreq/powernow-k8.c index 0023c7d..ffe89ba 100644 --- a/drivers/cpufreq/powernow-k8.c +++ b/drivers/cpufreq/powernow-k8.c @@ -1204,7 +1204,8 @@ out: }
static struct cpufreq_driver cpufreq_amd64_driver = {
- .flags = CPUFREQ_ASYNC_NOTIFICATION,
- .flags = CPUFREQ_ASYNC_NOTIFICATION |
CPUFREQ_SKIP_INITIAL_FREQ_CHECK,diff --git a/drivers/cpufreq/sc520_freq.c b/drivers/cpufreq/sc520_freq.c index 6adb354..d6a1cf6 100644 --- a/drivers/cpufreq/sc520_freq.c +++ b/drivers/cpufreq/sc520_freq.c @@ -89,6 +89,7 @@ static int sc520_freq_cpu_init(struct cpufreq_policy *policy)
static struct cpufreq_driver sc520_freq_driver = {
- .flags = CPUFREQ_SKIP_INITIAL_FREQ_CHECK, .get = sc520_freq_get_cpu_frequency, .verify = cpufreq_generic_frequency_table_verify, .target_index = sc520_freq_target,
diff --git a/drivers/cpufreq/speedstep-centrino.c b/drivers/cpufreq/speedstep-centrino.c index 4e1daca..887fd43 100644 --- a/drivers/cpufreq/speedstep-centrino.c +++ b/drivers/cpufreq/speedstep-centrino.c @@ -505,6 +505,7 @@ out: }
static struct cpufreq_driver centrino_driver = {
- .flags = CPUFREQ_SKIP_INITIAL_FREQ_CHECK, .name = "centrino", /* should be speedstep-centrino, but there's a 16 char limit */ .init = centrino_cpu_init,
diff --git a/drivers/cpufreq/speedstep-ich.c b/drivers/cpufreq/speedstep-ich.c index 7639b2b..67596e7 100644 --- a/drivers/cpufreq/speedstep-ich.c +++ b/drivers/cpufreq/speedstep-ich.c @@ -307,6 +307,7 @@ static int speedstep_cpu_init(struct cpufreq_policy *policy)
static struct cpufreq_driver speedstep_driver = {
- .flags = CPUFREQ_SKIP_INITIAL_FREQ_CHECK, .name = "speedstep-ich", .verify = cpufreq_generic_frequency_table_verify, .target_index = speedstep_target,
diff --git a/drivers/cpufreq/speedstep-smi.c b/drivers/cpufreq/speedstep-smi.c index 0f5326d..4f23289 100644 --- a/drivers/cpufreq/speedstep-smi.c +++ b/drivers/cpufreq/speedstep-smi.c @@ -308,6 +308,7 @@ static int speedstep_resume(struct cpufreq_policy *policy) }
static struct cpufreq_driver speedstep_driver = {
- .flags = CPUFREQ_SKIP_INITIAL_FREQ_CHECK, .name = "speedstep-smi", .verify = cpufreq_generic_frequency_table_verify, .target_index = speedstep_target,
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h index ee5fe9d..13b8802 100644 --- a/include/linux/cpufreq.h +++ b/include/linux/cpufreq.h @@ -252,6 +252,15 @@ struct cpufreq_driver { */ #define CPUFREQ_ASYNC_NOTIFICATION (1 << 4)
+/*
- Set by drivers which don't want cpufreq core to check if CPU is running at a
- frequency present in freq-table exposed by the driver. For other drivers if
- CPU is found running at an out of table freq, we will try to set it to a freq
- from the table. And if that fails, we will stop further boot process by
- issuing a BUG_ON().
- */
+#define CPUFREQ_SKIP_INITIAL_FREQ_CHECK (1 << 5)
- int cpufreq_register_driver(struct cpufreq_driver *driver_data); int cpufreq_unregister_driver(struct cpufreq_driver *driver_data);
On Monday, December 02, 2013 07:23:01 AM Dirk Brandewie wrote:
Sorry for late response to these threads. Holiday weekend in the US.
On 11/29/2013 06:14 AM, Viresh Kumar wrote:
On some systems we can't really say what frequency we're running at the moment and so for these we shouldn't check if we are running at a frequency present in frequency table. For now mark all x86 drivers with this flag: CPUFREQ_SKIP_INITIAL_FREQ_CHECK.
This patch also defines this macro. It will be used in cpufreq.c in the next patch.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org
V3->V4:
Reversed the order of patches
Moved definition of CPUFREQ_SKIP_INITIAL_FREQ_CHECK to patch 1/2 instead of 2/2.
drivers/cpufreq/acpi-cpufreq.c | 1 + drivers/cpufreq/cpufreq-nforce2.c | 1 + drivers/cpufreq/e_powersaver.c | 1 + drivers/cpufreq/elanfreq.c | 1 + drivers/cpufreq/gx-suspmod.c | 1 + drivers/cpufreq/ia64-acpi-cpufreq.c | 1 + drivers/cpufreq/intel_pstate.c | 2 +-
NAK for intel_pstate. The code that will use this flag also uses has_target() which is false for intel_pstate.
Is your point that intel_pstate doesn't need to have this flag set?
I think the logic for this patch is inverted. Why are we adding a flag to drivers where the bug cannot exist?
Well, enumerating goodness usually is a more sustainable strategy than enumerationg badness, especially if the latter is more abundant. :-)
I would be happier if the flag was CPUFREQ_NEED_INITIAL_FREQ_WORKAROUND to mark the platforms where the bug may exist let those platforms cary the flag for the next 10+ years.
I still believe that this bug should be dealt with in platform specific code since this is working around a bootloader bug. Are there platforms in the wild that even need this code or is this to support early platform bringup?
All it takes to introduce it is to use a buggy bootloader, so I guess there's a number of potentially affected platforms out there.
This really isn't about which individual platforms have the problem, but about which platforms have the property that the clock *can* be set to a wrong frequency initially. These are the ones we need to worry about regardless, because we can't control boot loaders and duplicating the same code in a number of drivers doesn't sound like a good approach.
So the only reasonable way forward I can see is to flag either the platforms that we need to worry about, or the platforms that we don't need to worry about and use that to decide whether or not to do the check.
Thanks, Rafael
On 3 December 2013 04:03, Rafael J. Wysocki rjw@rjwysocki.net wrote:
On Monday, December 02, 2013 07:23:01 AM Dirk Brandewie wrote:
Sorry for late response to these threads. Holiday weekend in the US.
No issues.
NAK for intel_pstate. The code that will use this flag also uses has_target() which is false for intel_pstate.
Is your point that intel_pstate doesn't need to have this flag set?
Probably yes as intel_pstate would never share freq-table. Will fix this.
I still believe that this bug should be dealt with in platform specific code since this is working around a bootloader bug. Are there platforms in the wild that even need this code or is this to support early platform bringup?
All it takes to introduce it is to use a buggy bootloader, so I guess there's a number of potentially affected platforms out there.
Yeah. This was first reported for OMAP SoC's though.
This really isn't about which individual platforms have the problem, but about which platforms have the property that the clock *can* be set to a wrong frequency initially. These are the ones we need to worry about regardless, because we can't control boot loaders and duplicating the same code in a number of drivers doesn't sound like a good approach.
So the only reasonable way forward I can see is to flag either the platforms that we need to worry about, or the platforms that we don't need to worry about and use that to decide whether or not to do the check.
To be honest Rafael suggested exactly this initially, i.e. set flag for all the drivers which want this to be done and I thought the number of drivers that don't want it would be lesser and so it would be easier to set this flag there.
But looking at the logic Dirk gave, it makes sense to do this for drivers which want this feature to be enabled. Moreover we really can't say for how long these drivers will continue keeping this flag as I don't really think its a bug. Kernel can't expect bootloaders to set core to a frequency which should be exactly same as what cpufreq table has. And so this flag may stay until cpufreq is around :)
I will send updated patches soon..
-- viresh
linaro-kernel@lists.linaro.org
-
Dirk Brandewie -
Rafael J. Wysocki -
Viresh Kumar