On 30.05.2014 22:13, Nishanth Menon wrote: > On 05/30/2014 03:02 PM, Tomasz Figa wrote: >> On 30.05.2014 21:50, Nishanth Menon wrote: >>> On 05/30/2014 01:55 PM, Mark Rutland wrote: >>>> On Fri, May 30, 2014 at 07:05:43PM +0100, Thomas Abraham wrote: >>>>> Hi Mark, >>>>> >>>>> On Fri, May 30, 2014 at 6:38 PM, Mark Rutland <mark.rutland@xxxxxxx> 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@xxxxxxxxxxx> >>>>>>> >>>>>>> Add a new optional boost-frequency binding for specifying the frequencies >>>>>>> usable in boost mode. >>>>>>> >>>>>>> Cc: Rob Herring <robh+dt@xxxxxxxxxx> >>>>>>> Cc: Pawel Moll <pawel.moll@xxxxxxx> >>>>>>> Cc: Mark Rutland <mark.rutland@xxxxxxx> >>>>>>> Cc: Ian Campbell <ijc+devicetree@xxxxxxxxxxxxxx> >>>>>>> Cc: Kumar Gala <galak@xxxxxxxxxxxxxx> >>>>>>> Signed-off-by: Thomas Abraham <thomas.ab@xxxxxxxxxxx> >>>>>>> Acked-by: Viresh Kumar <viresh.kumar@xxxxxxxxxx> >>>>>>> Acked-by: Nishanth Menon <nm@xxxxxx> >>>>>>> Acked-by: Lukasz Majewski <l.majewski@xxxxxxxxxxx> >>>>>>> --- >>>>>>> .../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. >>>>> >>>>> The mechanism of entry into and exit out of boost mode is outside the >>>>> scope of this documentation. >>>>> >>>>>> >>>>>> What are the limitations on boost frequencies? When is a CPU expected to >>>>>> go to these frequencies and for now long? When should I as a dt author >>>>>> place elements in boost-frequencies? >>>>> >>>>> I will add these details in the next revision of this patch. >>>> >>>> Cheers. >>>> >>>>>> >>>>>> Why are these in both operating-points and boost-frequencies? It'll be >>>>>> really easy to accidentally forget to mark something as a >>>>>> boost-frequency this way. Why not have a boost-points instead? >>>>> >>>>> Does boost-points mean a set of clock speeds which are not listed as >>>>> part of operating-points property? If yes, that also is a possible >>>>> implementation (it was implemented in one of the earlier version of >>>>> this series). Could you confirm that you want the boost mode speeds to >>>>> be exclusive of the speeds listed in operating-points? >>>> >>>> If these boost mode operating points are not generally advisable for use >>>> as the other operating-points are, then they should IMO been in an >>>> entirely separate property exclusive of (but in the same format as) the >>>> operating-points property, e.g. >>>> >>>> operating points = <A B>, <C D>; >>>> boost-points = <E F>; >>> >>> you are asking boost frequencies to remain for ever tied down to OPP >>> style description. >>> >>> What we are trying to describe? "What are my SoC's overclocked >>> frequencies"? That description can be used even in a system that does >>> not use OPP style table (say ACPI based OPP tables or whatever >>> acronymned table). >>> >>> Tying it down to operating points just because we have it today as a >>> convenient description, is limiting. >>> >>> Further, if we decide to educate boost-frequencies to also indicate >>> how long is it safe? That does indeed belong to boost-frequency >>> description and not OPP description. Or if we decide to change >>> operating-points description[1] in the future has an impact on >>> "boost-points" description, when it should not have. >>> >>>> >>>> Otherwise, without boost-mode support we have to parse the boost mode >>>> table to figure out which points to avoid. Or if someone typos a value >>> That is OS usage of h/w description - yeah - in anycase, if OS has no >>> ability to deal with boost-frequencies, it should skip it for sure. >>> >>>> in either table we might go into a boost mode when we didn't want to! >>>> >>> There are other ways to screw up device with dts typo. you could give >>> a wrong voltage(extra 0?) and ensure you never use the chip ever >>> again.. typos are dt bugs, we can do the best to write robust code to >>> report them. >>> >> >> Typos are not the primary thing to worry about here. Adding boost >> frequencies to the list of primary operating points is flawed, because >> an OS that has no idea of boost mode will use them as normal operating >> points and this is not desired. > > That means we have an implementation bug in OS since it does not > consider the complete hardware description that device tree is > providing. An analogy will be a regulator compatible match being used > but regulator-min-microvolt and regulator-max-microvolt being ignored > by OS. No. The operating points bindings were defined far before this boost-frequency and so there is no requirement to support the latter. > > We never said that "operating points" are "primary operating points". > all we said is they are "operating points" for the device - we dont > associate meanings to it. You may add to it[1] in platform code, as we > decided to. Maybe generic OPP bindings don't state that, but I believe that at least cpufreq-cpu0 bindings have been defined (and the driver implemented) this way and changing the semantics now would be breaking DT ABI compatibility. > > boost-frequencies are describing "overclocked frequencies" - should it > matter if the description of that comes from platform code OR existing > opp tables or what ever? I dont think defining them as "operating > point" style as the only way of describing overclocked frequencies are > the right approach to describing it. Nobody said that it is the only way. The whole point here is that it should be separate from the main operating-points property, as boost operating points are not normal operating points and the OS must be specifically aware how to handle them. Best regards, Tomasz -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
