Skip to contentSkip to conversation

cpufreq: opp: Add device tree based lookup of boost mode frequency

PATCH Superseded Thomas Abraham 5 participants 14 messages May 14, 2014

Subsystems (3)

v3 of 6 Superseded Latest is v6 →

lore
Share & tools
lore
#0 lore raw

v3 cover cpufreq: opp: Add device tree based lookup of boost mode frequency

Changes since v2:
- Reworked based on the "PM / OPP: move cpufreq specific helpers out of OPP layer" patch series posted by Nishanth Menon <[email protected]>.

Changes since v1:
- Boost mode frequencies are specfied as a set of frequencies instead of specifying them as OPPs. Thanks to Nishanth, Lukasz and Rob for the feedback.

Commit 6f19efc0 ("cpufreq: Add boost frequency support in core") adds support for CPU boost mode for CPUfreq drivers. To use the new boost mode, CPUfreq drivers have to specify the boost mode frequency and voltage within the CPUfreq driver, which is the case for Exynos4x12 CPUfreq driver.

But for CPUfreq drivers which obtain the OPPs from cpus node, this patch series adds support to specify boost mode frequencies in the cpu device tree node. This requirement came up when Lukasz pointed out the regression caused by the Exynos CPUfreq driver consolidation patches.

Thomas Abraham (2):
cpufreq / OPP: Allow boost frequency to be looked up from device tree
Documentation: devicetree: Add boost-frequency binding to list boost mode frequency

 .../devicetree/bindings/cpufreq/cpufreq-boost.txt  |   11 ++++++
 drivers/cpufreq/cpufreq_opp.c                      |   39 ++++++++++++++++++++
 2 files changed, 50 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt
Signature · 1 line
1.7.4.4
#1 lore raw

PATCH v3 1/2 cpufreq / OPP: Allow boost frequency to be looked up from device tree

From: Thomas Abraham <[email protected]>

Commit 6f19efc0 ("cpufreq: Add boost frequency support in core") adds support for CPU boost mode. This patch adds support for finding available boost frequencies from device tree and marking them as usable in boost mode.

Cc Nishanth Menon <[email protected]>
Cc Lukasz Majewski <[email protected]>
---
 drivers/cpufreq/cpufreq_opp.c |   39 +++++++++++++++++++++++++++++++++++++++
 1 files changed, 39 insertions(+), 0 deletions(-)
drivers/cpufreq/cpufreq_opp.c +39 -0
index c0c6f4a..e3c97f3 100644
@@ -19,6 +19,7 @@
 #include <linux/pm_opp.h>
 #include <linux/rcupdate.h>
 #include <linux/slab.h>
+#include <linux/of.h>
 
 /**
  * dev_pm_opp_init_cpufreq_table() - create a cpufreq table for a device
@@ -51,6 +52,10 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
 	struct cpufreq_frequency_table *freq_table = NULL;
 	int i, max_opps, ret = 0;
 	unsigned long rate;
+#ifdef CONFIG_CPU_FREQ_BOOST_SW
+	int j, len;
+	u32 *boost_freqs = NULL;
+#endif
 
 	rcu_read_lock();
 
@@ -82,6 +87,40 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
 
 	*table = &freq_table[0];
 
+#ifdef CONFIG_CPU_FREQ_BOOST_SW
+	if (of_find_property(dev->of_node, "boost-frequency", &len)) {
+		if (len == 0 || (len & (sizeof(u32) - 1)) != 0) {
+			dev_err(dev, "%s: invalid boost frequency\n", __func__);
+			ret = -EINVAL;
+			goto out;
+		}
+
+		boost_freqs = kzalloc(len, GFP_KERNEL);
+		if (!boost_freqs) {
+			dev_warn(dev, "%s: no memory for boost freq table\n",
+					__func__);
+			ret = -ENOMEM;
+			goto out;
+		}
+		of_property_read_u32_array(dev->of_node, "boost-frequency",
+			boost_freqs, len / sizeof(u32));
+	}
+
+	for (j = 0; j < len / sizeof(u32) && boost_freqs; j++) {
+		for (i = 0; freq_table[i].frequency != CPUFREQ_TABLE_END; i++) {
+			if (boost_freqs[j] == freq_table[i].frequency) {
+				freq_table[i].flags |= CPUFREQ_BOOST_FREQ;
+				break;
+			}
+		}
+		if (freq_table[i].frequency == CPUFREQ_TABLE_END)
+			pr_err("%s: invalid boost frequency %d\n",
+				__func__, boost_freqs[j]);
+	}
+
+	kfree(boost_freqs);
+#endif
+
 out:
 	rcu_read_unlock();
 	if (ret)
Signature · 1 line
1.7.4.4
maintainer
#2 lore raw

PATCH v3 1/2 cpufreq / OPP: Allow boost frequency to be looked up from device tree

Quoted 37 lines · Thomas Abraham

From: Thomas Abraham <[email protected]>

Commit 6f19efc0 ("cpufreq: Add boost frequency support in core") adds support for CPU boost mode. This patch adds support for finding available boost frequencies from device tree and marking them as usable in boost mode.

Cc Nishanth Menon <[email protected]>
Cc Lukasz Majewski <[email protected]>
---
 drivers/cpufreq/cpufreq_opp.c |   39 +++++++++++++++++++++++++++++++++++++++
 1 files changed, 39 insertions(+), 0 deletions(-)
drivers/cpufreq/cpufreq_opp.c +5 -0
index c0c6f4a..e3c97f3 100644
@@ -19,6 +19,7 @@
 #include <linux/pm_opp.h>
 #include <linux/rcupdate.h>
 #include <linux/slab.h>
+#include <linux/of.h>
 
 /**
  * dev_pm_opp_init_cpufreq_table() - create a cpufreq table for a device
@@ -51,6 +52,10 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
        struct cpufreq_frequency_table *freq_table = NULL;
        int i, max_opps, ret = 0;
        unsigned long rate;
+#ifdef CONFIG_CPU_FREQ_BOOST_SW
+       int j, len;
+       u32 *boost_freqs = NULL;
+#endif
 
        rcu_read_lock();
 

drivers/cpufreq/cpufreq_opp.c int dev_pm_opp_init_cpufreq_table(struct device *dev, · line 91

Quoted 5 lines
drivers/cpufreq/cpufreq_opp.c +2 -0
@@ -82,6 +87,40 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
 
        *table = &freq_table[0];
 
+#ifdef CONFIG_CPU_FREQ_BOOST_SW
+       if (of_find_property(dev->of_node, "boost-frequency", &len)) {

Does this mean another block inside the cpu node ? Like this: ?

cpu at 0 {
        compatible = "arm,cortex-a9";
        reg = <0>;
        next-level-cache = <&L2>;
        operating-points = <
                /* kHz    uV */
                792000  1100000
                396000  950000
                198000  850000
        >;
        boost-frequency = <
                792000
                198000
        >;
};

I think it we might better add another field in the opp block as these OPPs are rather boost one..

@Rob/Rafael: Opinion please ..

Quoted 8 lines
drivers/cpufreq/cpufreq_opp.c +8 -0
+               if (len == 0 || (len & (sizeof(u32) - 1)) != 0) {
+                       dev_err(dev, "%s: invalid boost frequency\n", __func__);
+                       ret = -EINVAL;
+                       goto out;
+               }
+
+               boost_freqs = kzalloc(len, GFP_KERNEL);
+               if (!boost_freqs) {

drivers/cpufreq/cpufreq_opp.c int dev_pm_opp_init_cpufreq_table(struct device *dev, · line 109

Quoted 10 lines
drivers/cpufreq/cpufreq_opp.c +10 -0
+                       dev_warn(dev, "%s: no memory for boost freq table\n",
+                                       __func__);
+                       ret = -ENOMEM;
+                       goto out;
+               }
+               of_property_read_u32_array(dev->of_node, "boost-frequency",
+                       boost_freqs, len / sizeof(u32));
+       }
+
+       for (j = 0; j < len / sizeof(u32) && boost_freqs; j++) {

Why is this present outside of above if {} ? as boost_freqs is guaranteed to be NULL without that.

drivers/cpufreq/cpufreq_opp.c int dev_pm_opp_init_cpufreq_table(struct device *dev, · line 111

Quoted 2 lines
drivers/cpufreq/cpufreq_opp.c +2 -0
+               for (i = 0; freq_table[i].frequency != CPUFREQ_TABLE_END; i++) {
+                       if (boost_freqs[j] == freq_table[i].frequency) {

Use cpufreq_frequency_table_get_index() instead.

Quoted 18 lines
drivers/cpufreq/cpufreq_opp.c +12 -1
+                               freq_table[i].flags |= CPUFREQ_BOOST_FREQ;
+                               break;
+                       }
+               }
+               if (freq_table[i].frequency == CPUFREQ_TABLE_END)
+                       pr_err("%s: invalid boost frequency %d\n",
+                               __func__, boost_freqs[j]);
+       }
+
+       kfree(boost_freqs);
+#endif
+
 out:
        rcu_read_unlock();
        if (ret)
--

1.7.4.4

#3 lore raw

PATCH v3 1/2 cpufreq / OPP: Allow boost frequency to be looked up from device tree

Quoted 1 line · Viresh Kumar

On 14 May 2014 06:32, Thomas Abraham <[email protected]> wrote:

[...]

Quoted 21 lines

drivers/cpufreq/cpufreq_opp.c int dev_pm_opp_init_cpufreq_table(struct device *dev, · line 91

Quoted 2 lines
drivers/cpufreq/cpufreq_opp.c +2 -0
+#ifdef CONFIG_CPU_FREQ_BOOST_SW
+       if (of_find_property(dev->of_node, "boost-frequency", &len)) {

Does this mean another block inside the cpu node ? Like this: ?

cpu at 0 {
        compatible = "arm,cortex-a9";
        reg = <0>;
        next-level-cache = <&L2>;
        operating-points = <
                /* kHz    uV */
                792000  1100000
                396000  950000
                198000  850000
        >;
        boost-frequency = <
                792000
                198000
        >;
};
Quoted 2 lines

I think it we might better add another field in the opp block as these OPPs are rather boost one..

If we change the meaning to be:

         operating-points = <
                 /* kHz    uV          boost? */
                 792000  1100000  1
                 396000  950000    0

We are adding a modifier to the OPP definition here and does imply every platform which may or maynot require "boost" need to indicate that - basically fails@least my "least common" description for a generic definition. As I had indicated in other threads -> we are back to the discussion of "additional properties" to an OPP.. Option 1:
- describe modifiers separately (as in boost-frequencies) - that
operate based on data from opp table.
Option 2:
- keep adding to the description of opp as time goes by - every SoC
has it's own set of quirks that are needed for an OPP - I know that at TI, we have certain OPPs that can only be enabled *if* "custom hardware driver" is enabled, and similar stories. - yet another example is enable the OPP if efuse X, bit Y is set.

Personally, looking at the various descriptions and bL, cpu-idle states dependencies on OPPs, etc.. Option 2 is a non-scalable approach.

[...]

#4 lore raw

PATCH v3 1/2 cpufreq / OPP: Allow boost frequency to be looked up from device tree

Hi Nishanth, Viresh

If I may add my 2 cents.

Quoted 10 lines
Quoted 1 line · Viresh Kumar

On 14 May 2014 06:32, Thomas Abraham <[email protected]> wrote:

[...]

Quoted 6 lines
Quoted 3 lines
(quoted diff) +2 -0
+#ifdef CONFIG_CPU_FREQ_BOOST_SW
+       if (of_find_property(dev->of_node, "boost-frequency",

&len)) {

Does this mean another block inside the cpu node ? Like this: ?

Quoted 14 lines
Quoted 6 lines
cpu at 0 {
        compatible = "arm,cortex-a9";
        reg = <0>;
        next-level-cache = <&L2>;
        operating-points = <
                /* kHz    uV */
Quoted 8 lines
                792000  1100000
                396000  950000
                198000  850000
        >;
        boost-frequency = <
                792000
                198000
        >;

I think that the above approach is more appealing [*].

Quoted 26 lines
Quoted 4 lines

};

I think it we might better add another field in the opp block as these OPPs are rather boost one..

If we change the meaning to be:

         operating-points = <
                 /* kHz    uV          boost? */
                 792000  1100000  1
                 396000  950000    0

We are adding a modifier to the OPP definition here and does imply every platform which may or maynot require "boost" need to indicate that - basically fails at least my "least common" description for a generic definition. As I had indicated in other threads -> we are back to the discussion of "additional properties" to an OPP.. Option 1:
- describe modifiers separately (as in boost-frequencies) - that
operate based on data from opp table.
Option 2:
- keep adding to the description of opp as time goes by - every SoC
has it's own set of quirks that are needed for an OPP - I know that at TI, we have certain OPPs that can only be enabled *if* "custom hardware driver" is enabled, and similar stories. - yet another example is enable the OPP if efuse X, bit Y is set.

Quoted 3 lines

Personally, looking at the various descriptions and bL, cpu-idle states dependencies on OPPs, etc.. Option 2 is a non-scalable approach.

I agree with Nishanth here, that point 1 (as described by Viresh at [*]) is a more scalable approach.

Quoted 2 lines

[...]

Signature · 5 lines
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
maintainer
#5 lore raw

PATCH v3 1/2 cpufreq / OPP: Allow boost frequency to be looked up from device tree

Quoted 2 lines · Lukasz Majewski

I agree with Nishanth here, that point 1 (as described by Viresh at [*]) is a more scalable approach.

The only reason why I wanted all that to be done at OPP level was to ensure if somebody else also needs it apart from cpufreq, they don't have to duplicate code and find it.. As it is present at a central place..

But if no other code is going to look for that, it may just be fine as is..

#6 lore raw

PATCH v3 1/2 cpufreq / OPP: Allow boost frequency to be looked up from device tree

Quoted 9 lines · Viresh Kumar
Quoted 2 lines · Lukasz Majewski

I agree with Nishanth here, that point 1 (as described by Viresh at [*]) is a more scalable approach.

The only reason why I wanted all that to be done at OPP level was to ensure if somebody else also needs it apart from cpufreq, they don't have to duplicate code and find it.. As it is present at a central place..

But if no other code is going to look for that, it may just be fine as is..

Yes, I agree with Nishanth and Lukasz for a separate binding.

#7 lore raw

PATCH v3 1/2 cpufreq / OPP: Allow boost frequency to be looked up from device tree

In reply to Viresh Kumar
Quoted 10 lines · Viresh Kumar
Quoted 2 lines · Lukasz Majewski

I agree with Nishanth here, that point 1 (as described by Viresh at [*]) is a more scalable approach.

The only reason why I wanted all that to be done at OPP level was to ensure if somebody else also needs it apart from cpufreq, they don't have to duplicate code and find it.. As it is present at a central place..

But if no other code is going to look for that, it may just be fine as is..

If we eventually have a need beyond cpufreq (say devfreq) with similar instances, then it makes sense to move it out to a generic place. Either way, code implementation/duplication is a OS problem - and should be looked at independent of the description in dts. If we feel the description is valid hardware description (which, personally, I do), then lets go to the next discussion point of where to put it - generic or cpufreq specific (here, I have no preference), and finally decide the implementation as necessary as a result of the description.

Signature · 2 lines
Regards,
Nishanth Menon
#8 lore raw

PATCH v3 1/2 cpufreq / OPP: Allow boost frequency to be looked up from device tree

In reply to Viresh Kumar
Quoted 80 lines · Viresh Kumar
Quoted 37 lines · Thomas Abraham

From: Thomas Abraham <[email protected]>

Commit 6f19efc0 ("cpufreq: Add boost frequency support in core") adds support for CPU boost mode. This patch adds support for finding available boost frequencies from device tree and marking them as usable in boost mode.

Cc Nishanth Menon <[email protected]>
Cc Lukasz Majewski <[email protected]>
---
 drivers/cpufreq/cpufreq_opp.c |   39 +++++++++++++++++++++++++++++++++++++++
 1 files changed, 39 insertions(+), 0 deletions(-)
drivers/cpufreq/cpufreq_opp.c +5 -0
index c0c6f4a..e3c97f3 100644
@@ -19,6 +19,7 @@
 #include <linux/pm_opp.h>
 #include <linux/rcupdate.h>
 #include <linux/slab.h>
+#include <linux/of.h>
 
 /**
  * dev_pm_opp_init_cpufreq_table() - create a cpufreq table for a device
@@ -51,6 +52,10 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
        struct cpufreq_frequency_table *freq_table = NULL;
        int i, max_opps, ret = 0;
        unsigned long rate;
+#ifdef CONFIG_CPU_FREQ_BOOST_SW
+       int j, len;
+       u32 *boost_freqs = NULL;
+#endif
 
        rcu_read_lock();
 

drivers/cpufreq/cpufreq_opp.c int dev_pm_opp_init_cpufreq_table(struct device *dev, · line 91

Quoted 5 lines
drivers/cpufreq/cpufreq_opp.c +2 -0
@@ -82,6 +87,40 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
 
        *table = &freq_table[0];
 
+#ifdef CONFIG_CPU_FREQ_BOOST_SW
+       if (of_find_property(dev->of_node, "boost-frequency", &len)) {

Does this mean another block inside the cpu node ? Like this: ?

cpu at 0 {
        compatible = "arm,cortex-a9";
        reg = <0>;
        next-level-cache = <&L2>;
        operating-points = <
                /* kHz    uV */
                792000  1100000
                396000  950000
                198000  850000
        >;
        boost-frequency = <
                792000
                198000
        >;
};

I think it we might better add another field in the opp block as these OPPs are rather boost one..

@Rob/Rafael: Opinion please ..

Quoted 13 lines
drivers/cpufreq/cpufreq_opp.c +13 -0
+               if (len == 0 || (len & (sizeof(u32) - 1)) != 0) {
+                       dev_err(dev, "%s: invalid boost frequency\n", __func__);
+                       ret = -EINVAL;
+                       goto out;
+               }
+
+               boost_freqs = kzalloc(len, GFP_KERNEL);
+               if (!boost_freqs) {
+                       dev_warn(dev, "%s: no memory for boost freq table\n",
+                                       __func__);
+                       ret = -ENOMEM;
+                       goto out;
+               }
Quoted 8 lines

drivers/cpufreq/cpufreq_opp.c int dev_pm_opp_init_cpufreq_table(struct device *dev, · line 109

Quoted 5 lines
drivers/cpufreq/cpufreq_opp.c +5 -0
+               of_property_read_u32_array(dev->of_node, "boost-frequency",
+                       boost_freqs, len / sizeof(u32));
+       }
+
+       for (j = 0; j < len / sizeof(u32) && boost_freqs; j++) {

Why is this present outside of above if {} ? as boost_freqs is guaranteed to be NULL without that.

Just to reduce indentation by one tab. No technical reasons. The code had to wrap at 80 column was becoming unreadable.

Quoted 5 lines

drivers/cpufreq/cpufreq_opp.c int dev_pm_opp_init_cpufreq_table(struct device *dev, · line 111

Quoted 3 lines
drivers/cpufreq/cpufreq_opp.c +2 -0
+               for (i = 0; freq_table[i].frequency != CPUFREQ_TABLE_END; i++) {
+                       if (boost_freqs[j] == freq_table[i].frequency) {

Use cpufreq_frequency_table_get_index() instead.

Okay. Thanks for pointing out.

Quoted 19 lines
Quoted 19 lines
drivers/cpufreq/cpufreq_opp.c +12 -1
+                               freq_table[i].flags |= CPUFREQ_BOOST_FREQ;
+                               break;
+                       }
+               }
+               if (freq_table[i].frequency == CPUFREQ_TABLE_END)
+                       pr_err("%s: invalid boost frequency %d\n",
+                               __func__, boost_freqs[j]);
+       }
+
+       kfree(boost_freqs);
+#endif
+
 out:
        rcu_read_unlock();
        if (ret)
--

1.7.4.4

maintainer
#9 lore raw

PATCH v3 1/2 cpufreq / OPP: Allow boost frequency to be looked up from device tree

Quoted 7 lines · Thomas Abraham
Quoted 2 lines · Viresh Kumar

Why is this present outside of above if {} ? as boost_freqs is guaranteed to be NULL without that.

Just to reduce indentation by one tab. No technical reasons. The code had to wrap at 80 column was becoming unreadable.

That's bad :), you have added an extra comparison just for that :( Instead kill the below indentation by doing a "goto out" with a ! of below expression:

Quoted 1 line
Quoted 1 line

+ if (of_find_property(dev->of_node, "boost-frequency", &len)) {

#10 lore raw

PATCH v3 2/2 Documentation: devicetree: Add boost-frequency binding to list boost mode frequency

In reply to Thomas Abraham

From: Thomas Abraham <[email protected]>

Add a new optional boost-frequency binding for specifying the frequencies usable in boost mode.

Cc Nishanth Menon <[email protected]>
Cc Lukasz Majewski <[email protected]>
Cc Rob Herring <[email protected]>
Cc Pawel Moll <[email protected]>
Cc Mark Rutland <[email protected]>
Cc Ian Campbell <[email protected]>
Cc Kumar Gala <[email protected]>
---
 .../devicetree/bindings/cpufreq/cpufreq-boost.txt  |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt
Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt new file +11 -0
new file mode 100644
index 0000000..d925e38
@@ -0,0 +1,11 @@
+* Device tree binding for CPU boost frequency (aka over-clocking)
+
+Certain CPU's can be operated in optional 'boost' mode (or sometimes referred as
+overclocking) in which the CPU can operate in frequencies beyond the normal
+operating conditions.
+
+Optional Properties:
+- boost-frequency: 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.
Signature · 1 line
1.7.4.4
#11 lore raw

PATCH v3 2/2 Documentation: devicetree: Add boost-frequency binding to list boost mode frequency

Quoted 24 lines · Thomas Abraham

From: Thomas Abraham <[email protected]>

Add a new optional boost-frequency binding for specifying the frequencies usable in boost mode.

Cc Nishanth Menon <[email protected]>
Cc Lukasz Majewski <[email protected]>
Cc Rob Herring <[email protected]>
Cc Pawel Moll <[email protected]>
Cc Mark Rutland <[email protected]>
Cc Ian Campbell <[email protected]>
Cc Kumar Gala <[email protected]>
---
 .../devicetree/bindings/cpufreq/cpufreq-boost.txt  |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt
Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt new file +0 -0
new file mode 100644
index 0000000..d925e38

Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt line 4

Quoted 4 lines
Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt new file +4 -0
@@ -0,0 +1,11 @@
+* Device tree binding for CPU boost frequency (aka over-clocking)
+
+Certain CPU's can be operated in optional 'boost' mode (or sometimes referred as
+overclocking) in which the CPU can operate in frequencies beyond the normal

"operate at"?

Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt line 5

Quoted 1 line
Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt new file +1 -0
+operating conditions.

"normal operating conditions" probably need a little bit of an expansion here perhaps?

Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt line 8

Quoted 3 lines
Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt new file +3 -0
+
+Optional Properties:
+- boost-frequency: list of frequencies in KHz to be used only in boost mode.

probably boost-frequencies?

Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt line 11

Quoted 3 lines
Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt new file +3 -0
+  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.

an example is expected here.

personally, I think I understand the intent here, but as a hardware description, will let folks comment if it is acceptable.

--
Regards,
Nishanth Menon

#12 lore raw

PATCH v3 2/2 Documentation: devicetree: Add boost-frequency binding to list boost mode frequency

Quoted 48 lines · Nishanth Menon
Quoted 24 lines · Thomas Abraham

From: Thomas Abraham <[email protected]>

Add a new optional boost-frequency binding for specifying the frequencies usable in boost mode.

Cc Nishanth Menon <[email protected]>
Cc Lukasz Majewski <[email protected]>
Cc Rob Herring <[email protected]>
Cc Pawel Moll <[email protected]>
Cc Mark Rutland <[email protected]>
Cc Ian Campbell <[email protected]>
Cc Kumar Gala <[email protected]>
---
 .../devicetree/bindings/cpufreq/cpufreq-boost.txt  |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt
Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt new file +0 -0
new file mode 100644
index 0000000..d925e38

Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt line 4

Quoted 4 lines
Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt new file +4 -0
@@ -0,0 +1,11 @@
+* Device tree binding for CPU boost frequency (aka over-clocking)
+
+Certain CPU's can be operated in optional 'boost' mode (or sometimes referred as
+overclocking) in which the CPU can operate in frequencies beyond the normal

"operate at"?

Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt line 5

Quoted 1 line
Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt new file +1 -0
+operating conditions.

"normal operating conditions" probably need a little bit of an expansion here perhaps?

Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt line 8

Quoted 3 lines
Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt new file +3 -0
+
+Optional Properties:
+- boost-frequency: list of frequencies in KHz to be used only in boost mode.

probably boost-frequencies?

Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt line 11

Quoted 3 lines
Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt new file +3 -0
+  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.
Quoted 4 lines

an example is expected here.

personally, I think I understand the intent here, but as a hardware description, will let folks comment if it is acceptable.

Thanks for your review Nishanth. I will do the changes you have suggested.

Regards,
Thomas.

Quoted 4 lines

--
Regards,
Nishanth Menon

#13 lore raw

PATCH v3 2/2 Documentation: devicetree: Add boost-frequency binding to list boost mode frequency

In reply to Thomas Abraham
Quoted 26 lines · Thomas Abraham

From: Thomas Abraham <[email protected]>

Add a new optional boost-frequency binding for specifying the frequencies usable in boost mode.

Cc Nishanth Menon <[email protected]>
Cc Lukasz Majewski <[email protected]>
Cc Rob Herring <[email protected]>
Cc Pawel Moll <[email protected]>
Cc Mark Rutland <[email protected]>
Cc Ian Campbell <[email protected]>
Cc Kumar Gala <[email protected]>
---
  .../devicetree/bindings/cpufreq/cpufreq-boost.txt  |   11 +++++++++++
  1 files changed, 11 insertions(+), 0 deletions(-)

create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt

Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt new file +1 -0
new file mode 100644
index 0000000..d925e38
@@ -0,0 +1,11 @@
+* Device tree binding for CPU boost frequency (aka over-clocking)

Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt line 11

Quoted 10 lines
Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt new file +10 -0
+
+Certain CPU's can be operated in optional 'boost' mode (or sometimes referred as
+overclocking) in which the CPU can operate in frequencies beyond the normal
+operating conditions.
+
+Optional Properties:
+- boost-frequency: 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.

Though I understand the need of it, I think the binding is designed to suffice the need of the boost frequency support in cpufreq.

Typically SoC would provide characteristics like under-drive(hits performance but most energy efficient), nominal(highest sustained performance w/o any external constraint like power, thermal) and over-drive(maximum performance but not sustainable for long periods)

IMO the binding could represent these unique points on the curve instead.

Regards,
Sudeep