Re: [PATCH 6/6 v6] cpufreq, highbank: add support for highbank cpufreq

Mark Langsdorf
2012-11-29T00:24:12Z

On 11/28/2012 03:05 PM, Mike Turquette wrote:
> Quoting Mark Langsdorf (2012-11-28 08:18:35)
>> On 11/28/2012 10:01 AM, Mike Turquette wrote:
>>> Quoting Shawn Guo (2012-11-28 07:17:44)
>>>> On Wed, Nov 28, 2012 at 10:58:02PM +0800, Shawn Guo wrote:
>>>>> On Wed, Nov 28, 2012 at 07:16:12AM -0600, Mark Langsdorf wrote:
>>>>>> I'd
>>>>>> have to move most of the logic of hb_set_target() into
>>>>>> clk_highbank.c:clk_pll_set_rate() and then add extra logic for when
>>>>>> cpufreq is not enabled/loaded.
>>>>>
>>>>> You only need to move hb_voltage_change() into cpu clock's .set_rate()
>>>>> hook with no need of checking if cpufreq is enabled or not.
>>>>>
>>>> Need to also check whether frequency or voltage should be changed first
>>>> in .set_rate() though.
>>>>
>>>> Shawn
>>>>
>>>
>>> The notifiers in the clk framework might be a better place for this than
>>> just simply hacking the logic into the .set_rate callback.
>>
>> Unless the clk notifiers are different than the cpufreq notifiers, they
>> don't handle returning error conditions very well. And given that the
>> voltage change operation can fail (though it almost always succeeds on a
>> retry) I need to be able to handle and detect that error condition.
>
> The notifier handler can handle the case where the transition fails (and
> needs to be retried).
>
> Also you should check out the clk notifiers. I think they handle
> failure decently. If a notifer returns an error code then everything
> unrolls and the clk_set_rate operation aborts.

Thanks for the pointer. The clk notifier calls seem to be working with
cpufreq-cpu0.

I did enough surgery on the code that I want to run a lot of stress
tests before I resubmit. I'll try to have something for Tuesday.

--Mark Langsdorf
Calxeda, Inc.