Re: [PATCH v6 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency

[Tomasz Figa <tomasz.figa@gmail.com>]

On 30.05.2014 20:33, Thomas Abraham wrote:
> On Fri, May 30, 2014 at 11:45 PM, Tomasz Figa <tomasz.figa@gmail.com> wrote:
>>
>>
>> On 30.05.2014 20:05, Thomas Abraham wrote:
>>> Hi Mark,
>>>
>>> On Fri, May 30, 2014 at 6:38 PM, Mark Rutland <mark.rutland@arm.com> wrote:
>>>> Hi,
>>>>
>>>> Apologies for being somewhat late w.r.t. review on this.
>>>>
>>>> On Fri, May 30, 2014 at 10:01:17AM +0100, Thomas Abraham wrote:
>>>>> From: Thomas Abraham <thomas.ab@samsung.com>
>>>>>
>>>>> Add a new optional boost-frequency binding for specifying the frequencies
>>>>> usable in boost mode.
>>>>>
>>>>> Cc: Rob Herring <robh+dt@kernel.org>
>>>>> Cc: Pawel Moll <pawel.moll@arm.com>
>>>>> Cc: Mark Rutland <mark.rutland@arm.com>
>>>>> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
>>>>> Cc: Kumar Gala <galak@codeaurora.org>
>>>>> Signed-off-by: Thomas Abraham <thomas.ab@samsung.com>
>>>>> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
>>>>> Acked-by: Nishanth Menon <nm@ti.com>
>>>>> Acked-by: Lukasz Majewski <l.majewski@samsung.com>
>>>>> ---
>>>>>  .../devicetree/bindings/cpufreq/cpufreq-boost.txt  |   38 ++++++++++++++++++++
>>>>>  1 file changed, 38 insertions(+)
>>>>>  create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt
>>>>> new file mode 100644
>>>>> index 0000000..63ed0fc
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt
>>>>> @@ -0,0 +1,38 @@
>>>>> +* Device tree binding for CPU boost frequency (aka over-clocking)
>>>>> +
>>>>> +Certain CPU's can be operated in optional 'boost' mode (or sometimes referred as
>>>>
>>>> Nit: CPUs (we're not greengrocers [1])
>>>>
>>>>> +overclocking) in which the CPU can operate at frequencies which are not
>>>>> +specified by the manufacturer as CPU's operating frequency.
>>>>> +
>>>>> +Optional Properties:
>>>>> +- boost-frequencies: list of frequencies in KHz to be used only in boost mode.
>>>>> +  This list should be a subset of frequencies listed in "operating-points"
>>>>> +  property. Refer to Documentation/devicetree/bindings/power/opp.txt for
>>>>> +  details about "operating-points" property.
>>>>
>>>> What is 'boost-mode'?
>>>
>>> boost-mode activates additional one or more cpu clock speeds (which
>>> are not specified as operating frequency of the cpu by the
>>> manufacturer). The cpu is allowed to operate in these boost mode
>>> speeds when the boost mode is active. The boost mode speeds are
>>> usually undocumented. Some of the chip samples could be clocked in
>>> boost mode speeds and only such samples can be safely operated in
>>> boost mode.
>>>
>>
>> IMHO the most important part that I believe should be stated in the
>> documentation is that CPU usually can operate in boost mode for limited
>> amount of time, which depends on thermal conditions, which makes the
>> boost operating points separate from normal ones, which can be used at
>> any time.
> 
> The factors that allow a CPU to operate in boost mode and the duration
> the CPU can operate in boost mode clock speed is a CPU specific
> description. This binding does not describe such CPU specific
> behavior. This binding only states the additional clock speeds CPU is
> allowed to operate in boost mode. The boost mode entry and exit
> conditions are implementation specific and not part of the scope of
> this binding and documentation.

The binding documentation describes properties related to boost mode and
so it must explain what boost mode is, otherwise those properties will
not make any sense. I'm not saying that specific constraints must be
listed, but the text should mention that those operating points might
need some.

Best regards,
Tomasz