Re: 答复: [PATCH] cpuidle: sirf : Add cpuidle for SiRFprimaII and SiRFatlasVI

[Daniel Lezcano <daniel.lezcano@linaro.org>]

On 10/08/2013 03:27 PM, Barry Song wrote:
>>>>> +       if (sirf_cpuidle.vcore_regulator)
>>>>> +               regulator_set_voltage(sirf_cpuidle.vcore_regulator,
>>>>> volt_old,
>>>>> +                               SIRFSOC_MAX_VOLTAGE);
>>>>> +
>>>>> +       clk_set_parent(sirf_cpuidle.cpu_clk, parent_clk);
>>>>> +       /* Todo: other C states */
>>>>
>>>>
>>>> It sounds very weird you have this freq/volt/opp code here.
>>>>
>>>> If you hit idle, the cpufreq driver didn't put the cpu in the state you
>>>> are trying to
>>>> bring here ?
>>>>
>>>> There isn't a power management unit on this board ?
>>>>
>>>
>>> By SiRF SoC, when enter idle mode, I set the freq to 26MHz and min
>>> voltage.
>>> That's will save the power consume and reduce leakage current.
>>
>>
>> Really ? :)
>>
>> You didn't answer the questions.
>>
>> 1. this code shouldn't be here but in cpufreq which is already the case with
>> the other patch you sent with this one. So I am convinced if you remove the
>> OPP here and let cpufreq to handle that, and keep the cpu_do_idle, you won't
>> see any difference with the current code.
>>
>> 2. Moreover, removing the OPP, there is only the WFI state which is the
>> default idle function. Hence the cpuidle driver itself has no interest and
>> you can simply remove it, except if you bring another idle state with the
>> driver.
>>
>> 3. More idle states are often handled through a PMU (Power Management Unit)
>> giving cpu power off, retention, cluster power down, etc ... Is there one on
>> this board ?
>>
>> The TRM can help to answer these questions.
>>
> Hi Daniel,
> here the codes actually want to put cpu into lowest freq and lowest
> voltage. because according to our test, power leakage will be much
> lower when cpu is in the status than we put it into WFI directly in a
> higher freq.
>
> here it is difficult to say cpufreq will do that automatically as it
> is decided by cpufreq policy.
>
> there is no power management unit which can put prima2 into a
> different idle status except WFI.
>
> any suggestion from you about this chip issue?

Not so much without further information. Definitively without the TRM it 
is hard to give any hints on how to go further.

Do you have the ability to do RAM self refresh ? If not, without a power 
management unit on your board, in my opinion you can drop the cpuidle 
driver because it means you can't do retention or poweroff the core.

Moreover, the OPP stuff, from a POV design, is tied with cpufreq and 
changing that from cpuidle will violate the decisions of the cpufreq 
governor.

Did you do some measurements without cpuidle but with the cpufreq 
governor 'ondemand' and compare that with the cpuidle driver you are 
submitting ?

Hopefully, Viresh (Cc'ed) can give its opinion.

Thanks
   -- Daniel


-- 
  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog