Sender : Inderpal Singh<inderpal.singh@xxxxxxxxxx> Date : 2012-04-10 19:05 (GMT+09:00) > > Hi MyungJoo, Hi Inderpal, > > On 4 April 2012 15:53, MyungJoo Ham wrote: > > The policy might have been changed since last call of target(). > > Thus, using cpufreq_frequency_table_target(), which depends on > > policy to find the correspoding index from a frequency, may return > > inconsistent index for freqs.old. Thus, old_index should be > > calculated not based on the current policy. > > > > We have been observing such issue when scaling_min/max_freq were > > updated and sometimes caused system lockups due to incorrectly > > configured voltages. > > > > Signed-off-by: MyungJoo Ham > > --- > > drivers/cpufreq/exynos-cpufreq.c | 13 +++++++++++-- > > 1 files changed, 11 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/cpufreq/exynos-cpufreq.c b/drivers/cpufreq/exynos-cpufreq.c > > index b243a7e..1577522 100644 > > --- a/drivers/cpufreq/exynos-cpufreq.c > > +++ b/drivers/cpufreq/exynos-cpufreq.c > > @@ -62,8 +62,17 @@ static int exynos_target(struct cpufreq_policy *policy, > > goto out; > > } > > > > - if (cpufreq_frequency_table_target(policy, freq_table, > > - freqs.old, relation, &old_index)) { > > + /* > > + * The policy may have been changed so that we cannot get proper > > + * old_index with cpufreq_frequency_table_target(). Thus, ignore > > + * policy and get the index from the raw frequency table. > > + */ > > + for (old_index = 0; > > + freq_table[old_index].frequency != CPUFREQ_TABLE_END; > > + old_index++) > > + if (freq_table[old_index].frequency == freqs.old) > > + break; > > + if (freq_table[old_index].frequency == CPUFREQ_TABLE_END) { > > ret = -EINVAL; > > goto out; > > } > > I also had the same issue and same fix while testing powertop 1.98 as > it changes the scaling_min/max_freq. > > The only concern I have is when this code gets called for the very > first time, the freqs.old will be the freq set by bootloader. Now if > bootloader sets a freq which is not in the freq_table (not sure if its > practical but theoretically its possible ), this code will error out. It is not practical (the vendor ships IPL), but theoretically, it's possible if one hacks the IPL. However, in such a theoretical scenario, cpufreq won't work anyway even if we accept out-of-specification freqs.old. In the case you've mentioned, the bootloader has configured the clock dividers and sources incorrectly (out of the chip vendor's specification). Then, the Exynos cpufreq is not compatible with such a bootloader anyway even if out-of-spec freqs.old is allowed. It is because Exynos cpufreq driver assumes that the dividers and sources are correctly configured previously; i.e., it does not reset/set every related divider or source when the frequency has been updated. Thus, the resulting frequency may be different from the intended frequency and often the system will suffer from lock ups. We've been observing lock ups when the bootloader configures some dividers different from what the kernel expects during suspend-to-ram and wakeup. Cheers! MyungJoo. > > > > -- > > 1.7.4.1 > > > > -- > > To unsubscribe from this list: send the line "unsubscribe cpufreq" in > > the body of a message to majordomo@xxxxxxxxxxxxxxx > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > Thanks, > Inder > > > > -- MyungJoo Ham (함명주), PHD System S/W Lab, S/W Platform Team, Software Center Samsung Electronics Cell: +82-10-6714-2858 ÿ淸º{.nÇ+돴윯돪†+%듚ÿ깁負¥Šwÿº{.nÇ+돴쑆俉z?왲^n‡r⊆¦zË곷h솳鈺Ú&{àz요z받쀺+€Ê+zf"·hš닱~넮녬iÿÿï곴ÿ묎çz_溫æj:+v돣þ)山øm