All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] Add device tree based lookup of boost mode frequency
@ 2014-02-07 15:19 ` Thomas Abraham
  0 siblings, 0 replies; 54+ messages in thread
From: Thomas Abraham @ 2014-02-07 15:19 UTC (permalink / raw)
  To: linux-pm, devicetree, linux-arm-kernel
  Cc: rjw, linux-samsung-soc, kgene.kim, t.figa, l.majewski,
	viresh.kumar, thomas.ab

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):
  PM / 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/base/power/opp.c                           |   34 +++++++++++++++++++-
 2 files changed, 44 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt


^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v2 0/2] Add device tree based lookup of boost mode frequency
@ 2014-02-07 15:19 ` Thomas Abraham
  0 siblings, 0 replies; 54+ messages in thread
From: Thomas Abraham @ 2014-02-07 15:19 UTC (permalink / raw)
  To: linux-arm-kernel

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):
  PM / 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/base/power/opp.c                           |   34 +++++++++++++++++++-
 2 files changed, 44 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v2 1/2] PM / OPP: Allow boost frequency to be looked up from device tree
  2014-02-07 15:19 ` Thomas Abraham
@ 2014-02-07 15:19   ` Thomas Abraham
  -1 siblings, 0 replies; 54+ messages in thread
From: Thomas Abraham @ 2014-02-07 15:19 UTC (permalink / raw)
  To: linux-pm, devicetree, linux-arm-kernel
  Cc: rjw, linux-samsung-soc, kgene.kim, t.figa, l.majewski,
	viresh.kumar, thomas.ab, Nishanth Menon

From: Thomas Abraham <thomas.ab@samsung.com>

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 <nm@ti.com>
Cc: Lukasz Majewski <l.majewski@samsung.com>
Signed-off-by: Thomas Abraham <thomas.ab@samsung.com>
---
 drivers/base/power/opp.c |   34 +++++++++++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
index fa41874..b636826 100644
--- a/drivers/base/power/opp.c
+++ b/drivers/base/power/opp.c
@@ -628,7 +628,8 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
 	struct device_opp *dev_opp;
 	struct dev_pm_opp *opp;
 	struct cpufreq_frequency_table *freq_table;
-	int i = 0;
+	int i = 0, j, len, ret;
+	u32 *boost_freqs = NULL;
 
 	/* Pretend as if I am an updater */
 	mutex_lock(&dev_opp_list_lock);
@@ -650,10 +651,35 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
 		return -ENOMEM;
 	}
 
+	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 err_boost;
+		}
+
+		boost_freqs = kzalloc(len, GFP_KERNEL);
+		if (!boost_freqs) {
+			dev_warn(dev, "%s: no memory for boost freq table\n",
+					__func__);
+			ret = -ENOMEM;
+			goto err_boost;
+		}
+		of_property_read_u32_array(dev->of_node, "boost-frequency",
+			boost_freqs, len / sizeof(u32));
+	}
+
 	list_for_each_entry(opp, &dev_opp->opp_list, node) {
 		if (opp->available) {
 			freq_table[i].driver_data = i;
 			freq_table[i].frequency = opp->rate / 1000;
+			for (j = 0; j < len / sizeof(u32) && boost_freqs; j++) {
+				if (boost_freqs[j] == freq_table[i].frequency) {
+					freq_table[i].driver_data =
+							CPUFREQ_BOOST_FREQ;
+					break;
+				}
+			}
 			i++;
 		}
 	}
@@ -663,8 +689,14 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
 	freq_table[i].frequency = CPUFREQ_TABLE_END;
 
 	*table = &freq_table[0];
+	kfree(boost_freqs);
 
 	return 0;
+
+err_boost:
+	kfree(freq_table);
+	mutex_unlock(&dev_opp_list_lock);
+	return ret;
 }
 EXPORT_SYMBOL_GPL(dev_pm_opp_init_cpufreq_table);
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* [PATCH v2 1/2] PM / OPP: Allow boost frequency to be looked up from device tree
@ 2014-02-07 15:19   ` Thomas Abraham
  0 siblings, 0 replies; 54+ messages in thread
From: Thomas Abraham @ 2014-02-07 15:19 UTC (permalink / raw)
  To: linux-arm-kernel

From: Thomas Abraham <thomas.ab@samsung.com>

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 <nm@ti.com>
Cc: Lukasz Majewski <l.majewski@samsung.com>
Signed-off-by: Thomas Abraham <thomas.ab@samsung.com>
---
 drivers/base/power/opp.c |   34 +++++++++++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
index fa41874..b636826 100644
--- a/drivers/base/power/opp.c
+++ b/drivers/base/power/opp.c
@@ -628,7 +628,8 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
 	struct device_opp *dev_opp;
 	struct dev_pm_opp *opp;
 	struct cpufreq_frequency_table *freq_table;
-	int i = 0;
+	int i = 0, j, len, ret;
+	u32 *boost_freqs = NULL;
 
 	/* Pretend as if I am an updater */
 	mutex_lock(&dev_opp_list_lock);
@@ -650,10 +651,35 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
 		return -ENOMEM;
 	}
 
+	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 err_boost;
+		}
+
+		boost_freqs = kzalloc(len, GFP_KERNEL);
+		if (!boost_freqs) {
+			dev_warn(dev, "%s: no memory for boost freq table\n",
+					__func__);
+			ret = -ENOMEM;
+			goto err_boost;
+		}
+		of_property_read_u32_array(dev->of_node, "boost-frequency",
+			boost_freqs, len / sizeof(u32));
+	}
+
 	list_for_each_entry(opp, &dev_opp->opp_list, node) {
 		if (opp->available) {
 			freq_table[i].driver_data = i;
 			freq_table[i].frequency = opp->rate / 1000;
+			for (j = 0; j < len / sizeof(u32) && boost_freqs; j++) {
+				if (boost_freqs[j] == freq_table[i].frequency) {
+					freq_table[i].driver_data =
+							CPUFREQ_BOOST_FREQ;
+					break;
+				}
+			}
 			i++;
 		}
 	}
@@ -663,8 +689,14 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
 	freq_table[i].frequency = CPUFREQ_TABLE_END;
 
 	*table = &freq_table[0];
+	kfree(boost_freqs);
 
 	return 0;
+
+err_boost:
+	kfree(freq_table);
+	mutex_unlock(&dev_opp_list_lock);
+	return ret;
 }
 EXPORT_SYMBOL_GPL(dev_pm_opp_init_cpufreq_table);
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
  2014-02-07 15:19 ` Thomas Abraham
@ 2014-02-07 15:19   ` Thomas Abraham
  -1 siblings, 0 replies; 54+ messages in thread
From: Thomas Abraham @ 2014-02-07 15:19 UTC (permalink / raw)
  To: linux-pm, devicetree, linux-arm-kernel
  Cc: rjw, linux-samsung-soc, kgene.kim, t.figa, l.majewski,
	viresh.kumar, thomas.ab, Nishanth Menon, Rob Herring, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala

From: Thomas Abraham <thomas.ab@samsung.com>

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

Cc: Nishanth Menon <nm@ti.com>
Cc: Lukasz Majewski <l.majewski@samsung.com>
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>
---
 Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt |   11 +++++++++++
 1 file changed, 11 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..d925e38
--- /dev/null
+++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt
@@ -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.
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 54+ messages in thread

* [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
@ 2014-02-07 15:19   ` Thomas Abraham
  0 siblings, 0 replies; 54+ messages in thread
From: Thomas Abraham @ 2014-02-07 15:19 UTC (permalink / raw)
  To: linux-arm-kernel

From: Thomas Abraham <thomas.ab@samsung.com>

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

Cc: Nishanth Menon <nm@ti.com>
Cc: Lukasz Majewski <l.majewski@samsung.com>
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>
---
 Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt |   11 +++++++++++
 1 file changed, 11 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..d925e38
--- /dev/null
+++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt
@@ -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.
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* Re: [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
  2014-02-07 15:19   ` Thomas Abraham
@ 2014-02-07 15:27     ` Nishanth Menon
  -1 siblings, 0 replies; 54+ messages in thread
From: Nishanth Menon @ 2014-02-07 15:27 UTC (permalink / raw)
  To: Thomas Abraham
  Cc: linux-pm, dt list, linux-arm-kernel, rjw, linux-samsung-soc,
	Kukjin Kim, Tomasz Figa, Lukasz Majewski, Viresh Kumar,
	thomas.ab, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala

On Fri, Feb 7, 2014 at 9:19 AM, Thomas Abraham <ta.omasab@gmail.com> wrote:
> From: Thomas Abraham <thomas.ab@samsung.com>
>
> Add a new optional boost-frequency binding for specifying the frequencies
> usable in boost mode.
>
> Cc: Nishanth Menon <nm@ti.com>
> Cc: Lukasz Majewski <l.majewski@samsung.com>
> 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>
> ---
>  Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt |   11 +++++++++++
>  1 file changed, 11 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..d925e38
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt
> @@ -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.

boost-frequencies?

> +  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.
> --
> 1.7.10.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
@ 2014-02-07 15:27     ` Nishanth Menon
  0 siblings, 0 replies; 54+ messages in thread
From: Nishanth Menon @ 2014-02-07 15:27 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Feb 7, 2014 at 9:19 AM, Thomas Abraham <ta.omasab@gmail.com> wrote:
> From: Thomas Abraham <thomas.ab@samsung.com>
>
> Add a new optional boost-frequency binding for specifying the frequencies
> usable in boost mode.
>
> Cc: Nishanth Menon <nm@ti.com>
> Cc: Lukasz Majewski <l.majewski@samsung.com>
> 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>
> ---
>  Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt |   11 +++++++++++
>  1 file changed, 11 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..d925e38
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt
> @@ -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.

boost-frequencies?

> +  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.
> --
> 1.7.10.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v2 1/2] PM / OPP: Allow boost frequency to be looked up from device tree
  2014-02-07 15:19   ` Thomas Abraham
@ 2014-02-07 15:29     ` Nishanth Menon
  -1 siblings, 0 replies; 54+ messages in thread
From: Nishanth Menon @ 2014-02-07 15:29 UTC (permalink / raw)
  To: Thomas Abraham
  Cc: linux-pm, dt list, linux-arm-kernel, rjw, linux-samsung-soc,
	Kukjin Kim, Tomasz Figa, Lukasz Majewski, Viresh Kumar,
	thomas.ab

On Fri, Feb 7, 2014 at 9:19 AM, Thomas Abraham <ta.omasab@gmail.com> wrote:
> From: Thomas Abraham <thomas.ab@samsung.com>
>
> 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 <nm@ti.com>
> Cc: Lukasz Majewski <l.majewski@samsung.com>
> Signed-off-by: Thomas Abraham <thomas.ab@samsung.com>
> ---
>  drivers/base/power/opp.c |   34 +++++++++++++++++++++++++++++++++-
>  1 file changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
> index fa41874..b636826 100644
> --- a/drivers/base/power/opp.c
> +++ b/drivers/base/power/opp.c
> @@ -628,7 +628,8 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
>         struct device_opp *dev_opp;
>         struct dev_pm_opp *opp;
>         struct cpufreq_frequency_table *freq_table;
> -       int i = 0;
> +       int i = 0, j, len, ret;
> +       u32 *boost_freqs = NULL;
>
>         /* Pretend as if I am an updater */
>         mutex_lock(&dev_opp_list_lock);
> @@ -650,10 +651,35 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
>                 return -ENOMEM;
>         }
>
> +       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 err_boost;
> +               }
> +
> +               boost_freqs = kzalloc(len, GFP_KERNEL);
> +               if (!boost_freqs) {
> +                       dev_warn(dev, "%s: no memory for boost freq table\n",
> +                                       __func__);
> +                       ret = -ENOMEM;
> +                       goto err_boost;
> +               }
> +               of_property_read_u32_array(dev->of_node, "boost-frequency",
> +                       boost_freqs, len / sizeof(u32));
> +       }
> +
>         list_for_each_entry(opp, &dev_opp->opp_list, node) {
>                 if (opp->available) {
>                         freq_table[i].driver_data = i;
>                         freq_table[i].frequency = opp->rate / 1000;
> +                       for (j = 0; j < len / sizeof(u32) && boost_freqs; j++) {
> +                               if (boost_freqs[j] == freq_table[i].frequency) {
> +                                       freq_table[i].driver_data =
> +                                                       CPUFREQ_BOOST_FREQ;
> +                                       break;
> +                               }
> +                       }

What if any one of the boost_freqs are not contained in the enabled frequencies?

>                         i++;
>                 }
>         }
> @@ -663,8 +689,14 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
>         freq_table[i].frequency = CPUFREQ_TABLE_END;
>
>         *table = &freq_table[0];
> +       kfree(boost_freqs);
>
>         return 0;
> +
> +err_boost:
> +       kfree(freq_table);
> +       mutex_unlock(&dev_opp_list_lock);
> +       return ret;
>  }
>  EXPORT_SYMBOL_GPL(dev_pm_opp_init_cpufreq_table);
>
> --
> 1.7.10.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v2 1/2] PM / OPP: Allow boost frequency to be looked up from device tree
@ 2014-02-07 15:29     ` Nishanth Menon
  0 siblings, 0 replies; 54+ messages in thread
From: Nishanth Menon @ 2014-02-07 15:29 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Feb 7, 2014 at 9:19 AM, Thomas Abraham <ta.omasab@gmail.com> wrote:
> From: Thomas Abraham <thomas.ab@samsung.com>
>
> 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 <nm@ti.com>
> Cc: Lukasz Majewski <l.majewski@samsung.com>
> Signed-off-by: Thomas Abraham <thomas.ab@samsung.com>
> ---
>  drivers/base/power/opp.c |   34 +++++++++++++++++++++++++++++++++-
>  1 file changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
> index fa41874..b636826 100644
> --- a/drivers/base/power/opp.c
> +++ b/drivers/base/power/opp.c
> @@ -628,7 +628,8 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
>         struct device_opp *dev_opp;
>         struct dev_pm_opp *opp;
>         struct cpufreq_frequency_table *freq_table;
> -       int i = 0;
> +       int i = 0, j, len, ret;
> +       u32 *boost_freqs = NULL;
>
>         /* Pretend as if I am an updater */
>         mutex_lock(&dev_opp_list_lock);
> @@ -650,10 +651,35 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
>                 return -ENOMEM;
>         }
>
> +       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 err_boost;
> +               }
> +
> +               boost_freqs = kzalloc(len, GFP_KERNEL);
> +               if (!boost_freqs) {
> +                       dev_warn(dev, "%s: no memory for boost freq table\n",
> +                                       __func__);
> +                       ret = -ENOMEM;
> +                       goto err_boost;
> +               }
> +               of_property_read_u32_array(dev->of_node, "boost-frequency",
> +                       boost_freqs, len / sizeof(u32));
> +       }
> +
>         list_for_each_entry(opp, &dev_opp->opp_list, node) {
>                 if (opp->available) {
>                         freq_table[i].driver_data = i;
>                         freq_table[i].frequency = opp->rate / 1000;
> +                       for (j = 0; j < len / sizeof(u32) && boost_freqs; j++) {
> +                               if (boost_freqs[j] == freq_table[i].frequency) {
> +                                       freq_table[i].driver_data =
> +                                                       CPUFREQ_BOOST_FREQ;
> +                                       break;
> +                               }
> +                       }

What if any one of the boost_freqs are not contained in the enabled frequencies?

>                         i++;
>                 }
>         }
> @@ -663,8 +689,14 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
>         freq_table[i].frequency = CPUFREQ_TABLE_END;
>
>         *table = &freq_table[0];
> +       kfree(boost_freqs);
>
>         return 0;
> +
> +err_boost:
> +       kfree(freq_table);
> +       mutex_unlock(&dev_opp_list_lock);
> +       return ret;
>  }
>  EXPORT_SYMBOL_GPL(dev_pm_opp_init_cpufreq_table);
>
> --
> 1.7.10.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v2 1/2] PM / OPP: Allow boost frequency to be looked up from device tree
  2014-02-07 15:29     ` Nishanth Menon
@ 2014-02-07 15:38       ` Thomas Abraham
  -1 siblings, 0 replies; 54+ messages in thread
From: Thomas Abraham @ 2014-02-07 15:38 UTC (permalink / raw)
  To: Nishanth Menon
  Cc: linux-pm, dt list, linux-arm-kernel, rjw, linux-samsung-soc,
	Kukjin Kim, Tomasz Figa, Lukasz Majewski, Viresh Kumar,
	thomas.ab

On Fri, Feb 7, 2014 at 8:59 PM, Nishanth Menon <nm@ti.com> wrote:
> On Fri, Feb 7, 2014 at 9:19 AM, Thomas Abraham <ta.omasab@gmail.com> wrote:
>> From: Thomas Abraham <thomas.ab@samsung.com>
>>
>> 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 <nm@ti.com>
>> Cc: Lukasz Majewski <l.majewski@samsung.com>
>> Signed-off-by: Thomas Abraham <thomas.ab@samsung.com>
>> ---
>>  drivers/base/power/opp.c |   34 +++++++++++++++++++++++++++++++++-
>>  1 file changed, 33 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
>> index fa41874..b636826 100644
>> --- a/drivers/base/power/opp.c
>> +++ b/drivers/base/power/opp.c
>> @@ -628,7 +628,8 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
>>         struct device_opp *dev_opp;
>>         struct dev_pm_opp *opp;
>>         struct cpufreq_frequency_table *freq_table;
>> -       int i = 0;
>> +       int i = 0, j, len, ret;
>> +       u32 *boost_freqs = NULL;
>>
>>         /* Pretend as if I am an updater */
>>         mutex_lock(&dev_opp_list_lock);
>> @@ -650,10 +651,35 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
>>                 return -ENOMEM;
>>         }
>>
>> +       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 err_boost;
>> +               }
>> +
>> +               boost_freqs = kzalloc(len, GFP_KERNEL);
>> +               if (!boost_freqs) {
>> +                       dev_warn(dev, "%s: no memory for boost freq table\n",
>> +                                       __func__);
>> +                       ret = -ENOMEM;
>> +                       goto err_boost;
>> +               }
>> +               of_property_read_u32_array(dev->of_node, "boost-frequency",
>> +                       boost_freqs, len / sizeof(u32));
>> +       }
>> +
>>         list_for_each_entry(opp, &dev_opp->opp_list, node) {
>>                 if (opp->available) {
>>                         freq_table[i].driver_data = i;
>>                         freq_table[i].frequency = opp->rate / 1000;
>> +                       for (j = 0; j < len / sizeof(u32) && boost_freqs; j++) {
>> +                               if (boost_freqs[j] == freq_table[i].frequency) {
>> +                                       freq_table[i].driver_data =
>> +                                                       CPUFREQ_BOOST_FREQ;
>> +                                       break;
>> +                               }
>> +                       }
>
> What if any one of the boost_freqs are not contained in the enabled frequencies?

It is not used as a boost frequency because its corresponding voltage
is not known. If required a warning can be printed out for the same.

Thanks,
Thomas.

>
>>                         i++;
>>                 }
>>         }
>> @@ -663,8 +689,14 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
>>         freq_table[i].frequency = CPUFREQ_TABLE_END;
>>
>>         *table = &freq_table[0];
>> +       kfree(boost_freqs);
>>
>>         return 0;
>> +
>> +err_boost:
>> +       kfree(freq_table);
>> +       mutex_unlock(&dev_opp_list_lock);
>> +       return ret;
>>  }
>>  EXPORT_SYMBOL_GPL(dev_pm_opp_init_cpufreq_table);
>>
>> --
>> 1.7.10.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v2 1/2] PM / OPP: Allow boost frequency to be looked up from device tree
@ 2014-02-07 15:38       ` Thomas Abraham
  0 siblings, 0 replies; 54+ messages in thread
From: Thomas Abraham @ 2014-02-07 15:38 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Feb 7, 2014 at 8:59 PM, Nishanth Menon <nm@ti.com> wrote:
> On Fri, Feb 7, 2014 at 9:19 AM, Thomas Abraham <ta.omasab@gmail.com> wrote:
>> From: Thomas Abraham <thomas.ab@samsung.com>
>>
>> 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 <nm@ti.com>
>> Cc: Lukasz Majewski <l.majewski@samsung.com>
>> Signed-off-by: Thomas Abraham <thomas.ab@samsung.com>
>> ---
>>  drivers/base/power/opp.c |   34 +++++++++++++++++++++++++++++++++-
>>  1 file changed, 33 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
>> index fa41874..b636826 100644
>> --- a/drivers/base/power/opp.c
>> +++ b/drivers/base/power/opp.c
>> @@ -628,7 +628,8 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
>>         struct device_opp *dev_opp;
>>         struct dev_pm_opp *opp;
>>         struct cpufreq_frequency_table *freq_table;
>> -       int i = 0;
>> +       int i = 0, j, len, ret;
>> +       u32 *boost_freqs = NULL;
>>
>>         /* Pretend as if I am an updater */
>>         mutex_lock(&dev_opp_list_lock);
>> @@ -650,10 +651,35 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
>>                 return -ENOMEM;
>>         }
>>
>> +       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 err_boost;
>> +               }
>> +
>> +               boost_freqs = kzalloc(len, GFP_KERNEL);
>> +               if (!boost_freqs) {
>> +                       dev_warn(dev, "%s: no memory for boost freq table\n",
>> +                                       __func__);
>> +                       ret = -ENOMEM;
>> +                       goto err_boost;
>> +               }
>> +               of_property_read_u32_array(dev->of_node, "boost-frequency",
>> +                       boost_freqs, len / sizeof(u32));
>> +       }
>> +
>>         list_for_each_entry(opp, &dev_opp->opp_list, node) {
>>                 if (opp->available) {
>>                         freq_table[i].driver_data = i;
>>                         freq_table[i].frequency = opp->rate / 1000;
>> +                       for (j = 0; j < len / sizeof(u32) && boost_freqs; j++) {
>> +                               if (boost_freqs[j] == freq_table[i].frequency) {
>> +                                       freq_table[i].driver_data =
>> +                                                       CPUFREQ_BOOST_FREQ;
>> +                                       break;
>> +                               }
>> +                       }
>
> What if any one of the boost_freqs are not contained in the enabled frequencies?

It is not used as a boost frequency because its corresponding voltage
is not known. If required a warning can be printed out for the same.

Thanks,
Thomas.

>
>>                         i++;
>>                 }
>>         }
>> @@ -663,8 +689,14 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
>>         freq_table[i].frequency = CPUFREQ_TABLE_END;
>>
>>         *table = &freq_table[0];
>> +       kfree(boost_freqs);
>>
>>         return 0;
>> +
>> +err_boost:
>> +       kfree(freq_table);
>> +       mutex_unlock(&dev_opp_list_lock);
>> +       return ret;
>>  }
>>  EXPORT_SYMBOL_GPL(dev_pm_opp_init_cpufreq_table);
>>
>> --
>> 1.7.10.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
  2014-02-07 15:27     ` Nishanth Menon
@ 2014-02-07 15:38       ` Thomas Abraham
  -1 siblings, 0 replies; 54+ messages in thread
From: Thomas Abraham @ 2014-02-07 15:38 UTC (permalink / raw)
  To: Nishanth Menon
  Cc: linux-pm, dt list, linux-arm-kernel, rjw, linux-samsung-soc,
	Kukjin Kim, Tomasz Figa, Lukasz Majewski, Viresh Kumar,
	thomas.ab, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala

On Fri, Feb 7, 2014 at 8:57 PM, Nishanth Menon <nm@ti.com> wrote:
> On Fri, Feb 7, 2014 at 9:19 AM, Thomas Abraham <ta.omasab@gmail.com> wrote:
>> From: Thomas Abraham <thomas.ab@samsung.com>
>>
>> Add a new optional boost-frequency binding for specifying the frequencies
>> usable in boost mode.
>>
>> Cc: Nishanth Menon <nm@ti.com>
>> Cc: Lukasz Majewski <l.majewski@samsung.com>
>> 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>
>> ---
>>  Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt |   11 +++++++++++
>>  1 file changed, 11 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..d925e38
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt
>> @@ -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.
>
> boost-frequencies?

Okay, I will change it.

Thanks,
Thomas.

>
>> +  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.
>> --
>> 1.7.10.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
@ 2014-02-07 15:38       ` Thomas Abraham
  0 siblings, 0 replies; 54+ messages in thread
From: Thomas Abraham @ 2014-02-07 15:38 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Feb 7, 2014 at 8:57 PM, Nishanth Menon <nm@ti.com> wrote:
> On Fri, Feb 7, 2014 at 9:19 AM, Thomas Abraham <ta.omasab@gmail.com> wrote:
>> From: Thomas Abraham <thomas.ab@samsung.com>
>>
>> Add a new optional boost-frequency binding for specifying the frequencies
>> usable in boost mode.
>>
>> Cc: Nishanth Menon <nm@ti.com>
>> Cc: Lukasz Majewski <l.majewski@samsung.com>
>> 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>
>> ---
>>  Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt |   11 +++++++++++
>>  1 file changed, 11 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..d925e38
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt
>> @@ -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.
>
> boost-frequencies?

Okay, I will change it.

Thanks,
Thomas.

>
>> +  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.
>> --
>> 1.7.10.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v2 1/2] PM / OPP: Allow boost frequency to be looked up from device tree
  2014-02-07 15:38       ` Thomas Abraham
@ 2014-02-07 15:52         ` Nishanth Menon
  -1 siblings, 0 replies; 54+ messages in thread
From: Nishanth Menon @ 2014-02-07 15:52 UTC (permalink / raw)
  To: Thomas Abraham
  Cc: dt list, Lukasz Majewski, linux-samsung-soc, linux-pm,
	Viresh Kumar, Tomasz Figa, rjw, Kukjin Kim, thomas.ab,
	linux-arm-kernel

On 02/07/2014 09:38 AM, Thomas Abraham wrote:
[...]
>>> diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
>>> index fa41874..b636826 100644
>>> --- a/drivers/base/power/opp.c
>>> +++ b/drivers/base/power/opp.c
>>> @@ -628,7 +628,8 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
>>>         struct device_opp *dev_opp;
>>>         struct dev_pm_opp *opp;
>>>         struct cpufreq_frequency_table *freq_table;
>>> -       int i = 0;
>>> +       int i = 0, j, len, ret;
>>> +       u32 *boost_freqs = NULL;
>>>
>>>         /* Pretend as if I am an updater */
>>>         mutex_lock(&dev_opp_list_lock);
>>> @@ -650,10 +651,35 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
>>>                 return -ENOMEM;
>>>         }
>>>
>>> +       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 err_boost;
>>> +               }
>>> +
>>> +               boost_freqs = kzalloc(len, GFP_KERNEL);
>>> +               if (!boost_freqs) {
>>> +                       dev_warn(dev, "%s: no memory for boost freq table\n",
>>> +                                       __func__);
>>> +                       ret = -ENOMEM;
>>> +                       goto err_boost;
>>> +               }
>>> +               of_property_read_u32_array(dev->of_node, "boost-frequency",
>>> +                       boost_freqs, len / sizeof(u32));
>>> +       }
>>> +
>>>         list_for_each_entry(opp, &dev_opp->opp_list, node) {
>>>                 if (opp->available) {
>>>                         freq_table[i].driver_data = i;
>>>                         freq_table[i].frequency = opp->rate / 1000;
>>> +                       for (j = 0; j < len / sizeof(u32) && boost_freqs; j++) {
>>> +                               if (boost_freqs[j] == freq_table[i].frequency) {
>>> +                                       freq_table[i].driver_data =
>>> +                                                       CPUFREQ_BOOST_FREQ;
>>> +                                       break;
>>> +                               }
>>> +                       }
>>
>> What if any one of the boost_freqs are not contained in the enabled frequencies?
> 
> It is not used as a boost frequency because its corresponding voltage
> is not known. If required a warning can be printed out for the same.

yes - that would be good, as it helps debug if there are developer
errors in dts.


-- 
Regards,
Nishanth Menon

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v2 1/2] PM / OPP: Allow boost frequency to be looked up from device tree
@ 2014-02-07 15:52         ` Nishanth Menon
  0 siblings, 0 replies; 54+ messages in thread
From: Nishanth Menon @ 2014-02-07 15:52 UTC (permalink / raw)
  To: linux-arm-kernel

On 02/07/2014 09:38 AM, Thomas Abraham wrote:
[...]
>>> diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
>>> index fa41874..b636826 100644
>>> --- a/drivers/base/power/opp.c
>>> +++ b/drivers/base/power/opp.c
>>> @@ -628,7 +628,8 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
>>>         struct device_opp *dev_opp;
>>>         struct dev_pm_opp *opp;
>>>         struct cpufreq_frequency_table *freq_table;
>>> -       int i = 0;
>>> +       int i = 0, j, len, ret;
>>> +       u32 *boost_freqs = NULL;
>>>
>>>         /* Pretend as if I am an updater */
>>>         mutex_lock(&dev_opp_list_lock);
>>> @@ -650,10 +651,35 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
>>>                 return -ENOMEM;
>>>         }
>>>
>>> +       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 err_boost;
>>> +               }
>>> +
>>> +               boost_freqs = kzalloc(len, GFP_KERNEL);
>>> +               if (!boost_freqs) {
>>> +                       dev_warn(dev, "%s: no memory for boost freq table\n",
>>> +                                       __func__);
>>> +                       ret = -ENOMEM;
>>> +                       goto err_boost;
>>> +               }
>>> +               of_property_read_u32_array(dev->of_node, "boost-frequency",
>>> +                       boost_freqs, len / sizeof(u32));
>>> +       }
>>> +
>>>         list_for_each_entry(opp, &dev_opp->opp_list, node) {
>>>                 if (opp->available) {
>>>                         freq_table[i].driver_data = i;
>>>                         freq_table[i].frequency = opp->rate / 1000;
>>> +                       for (j = 0; j < len / sizeof(u32) && boost_freqs; j++) {
>>> +                               if (boost_freqs[j] == freq_table[i].frequency) {
>>> +                                       freq_table[i].driver_data =
>>> +                                                       CPUFREQ_BOOST_FREQ;
>>> +                                       break;
>>> +                               }
>>> +                       }
>>
>> What if any one of the boost_freqs are not contained in the enabled frequencies?
> 
> It is not used as a boost frequency because its corresponding voltage
> is not known. If required a warning can be printed out for the same.

yes - that would be good, as it helps debug if there are developer
errors in dts.


-- 
Regards,
Nishanth Menon

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v2 1/2] PM / OPP: Allow boost frequency to be looked up from device tree
  2014-02-07 15:19   ` Thomas Abraham
@ 2014-02-07 16:01     ` Sudeep Holla
  -1 siblings, 0 replies; 54+ messages in thread
From: Sudeep Holla @ 2014-02-07 16:01 UTC (permalink / raw)
  To: Thomas Abraham, linux-pm, devicetree, linux-arm-kernel
  Cc: Sudeep.Holla, rjw, linux-samsung-soc, kgene.kim, t.figa,
	l.majewski, viresh.kumar, thomas.ab, Nishanth Menon

On 07/02/14 15:19, Thomas Abraham wrote:
> From: Thomas Abraham <thomas.ab@samsung.com>
> 
> 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 <nm@ti.com>
> Cc: Lukasz Majewski <l.majewski@samsung.com>
> Signed-off-by: Thomas Abraham <thomas.ab@samsung.com>
> ---
>  drivers/base/power/opp.c |   34 +++++++++++++++++++++++++++++++++-
>  1 file changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
> index fa41874..b636826 100644
> --- a/drivers/base/power/opp.c
> +++ b/drivers/base/power/opp.c
> @@ -628,7 +628,8 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
>  	struct device_opp *dev_opp;
>  	struct dev_pm_opp *opp;
>  	struct cpufreq_frequency_table *freq_table;
> -	int i = 0;
> +	int i = 0, j, len, ret;
> +	u32 *boost_freqs = NULL;
>  
>  	/* Pretend as if I am an updater */
>  	mutex_lock(&dev_opp_list_lock);
> @@ -650,10 +651,35 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
>  		return -ENOMEM;
>  	}
>  
> +	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 err_boost;
> +		}
> +
> +		boost_freqs = kzalloc(len, GFP_KERNEL);
> +		if (!boost_freqs) {
> +			dev_warn(dev, "%s: no memory for boost freq table\n",
> +					__func__);
> +			ret = -ENOMEM;
> +			goto err_boost;
> +		}
> +		of_property_read_u32_array(dev->of_node, "boost-frequency",
> +			boost_freqs, len / sizeof(u32));
> +	}
> +
>  	list_for_each_entry(opp, &dev_opp->opp_list, node) {
>  		if (opp->available) {
>  			freq_table[i].driver_data = i;
>  			freq_table[i].frequency = opp->rate / 1000;
> +			for (j = 0; j < len / sizeof(u32) && boost_freqs; j++) {
> +				if (boost_freqs[j] == freq_table[i].frequency) {
> +					freq_table[i].driver_data =
> +							CPUFREQ_BOOST_FREQ;
> +					break;
> +				}
> +			}
>  			i++;
>  		}
>  	}
IIRC you had mentioned that the boost-opp was not limited to be a cpufreq, but
this change seems to be cpufreq only.

Regards,
Sudeep

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v2 1/2] PM / OPP: Allow boost frequency to be looked up from device tree
@ 2014-02-07 16:01     ` Sudeep Holla
  0 siblings, 0 replies; 54+ messages in thread
From: Sudeep Holla @ 2014-02-07 16:01 UTC (permalink / raw)
  To: linux-arm-kernel

On 07/02/14 15:19, Thomas Abraham wrote:
> From: Thomas Abraham <thomas.ab@samsung.com>
> 
> 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 <nm@ti.com>
> Cc: Lukasz Majewski <l.majewski@samsung.com>
> Signed-off-by: Thomas Abraham <thomas.ab@samsung.com>
> ---
>  drivers/base/power/opp.c |   34 +++++++++++++++++++++++++++++++++-
>  1 file changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
> index fa41874..b636826 100644
> --- a/drivers/base/power/opp.c
> +++ b/drivers/base/power/opp.c
> @@ -628,7 +628,8 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
>  	struct device_opp *dev_opp;
>  	struct dev_pm_opp *opp;
>  	struct cpufreq_frequency_table *freq_table;
> -	int i = 0;
> +	int i = 0, j, len, ret;
> +	u32 *boost_freqs = NULL;
>  
>  	/* Pretend as if I am an updater */
>  	mutex_lock(&dev_opp_list_lock);
> @@ -650,10 +651,35 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
>  		return -ENOMEM;
>  	}
>  
> +	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 err_boost;
> +		}
> +
> +		boost_freqs = kzalloc(len, GFP_KERNEL);
> +		if (!boost_freqs) {
> +			dev_warn(dev, "%s: no memory for boost freq table\n",
> +					__func__);
> +			ret = -ENOMEM;
> +			goto err_boost;
> +		}
> +		of_property_read_u32_array(dev->of_node, "boost-frequency",
> +			boost_freqs, len / sizeof(u32));
> +	}
> +
>  	list_for_each_entry(opp, &dev_opp->opp_list, node) {
>  		if (opp->available) {
>  			freq_table[i].driver_data = i;
>  			freq_table[i].frequency = opp->rate / 1000;
> +			for (j = 0; j < len / sizeof(u32) && boost_freqs; j++) {
> +				if (boost_freqs[j] == freq_table[i].frequency) {
> +					freq_table[i].driver_data =
> +							CPUFREQ_BOOST_FREQ;
> +					break;
> +				}
> +			}
>  			i++;
>  		}
>  	}
IIRC you had mentioned that the boost-opp was not limited to be a cpufreq, but
this change seems to be cpufreq only.

Regards,
Sudeep

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
  2014-02-07 15:19   ` Thomas Abraham
@ 2014-02-07 16:15     ` Sudeep Holla
  -1 siblings, 0 replies; 54+ messages in thread
From: Sudeep Holla @ 2014-02-07 16:15 UTC (permalink / raw)
  To: Thomas Abraham, linux-pm, devicetree, linux-arm-kernel
  Cc: Sudeep.Holla, rjw, linux-samsung-soc, kgene.kim, t.figa,
	l.majewski, viresh.kumar, thomas.ab, Nishanth Menon, Rob Herring,
	Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala

On 07/02/14 15:19, 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: Nishanth Menon <nm@ti.com>
> Cc: Lukasz Majewski <l.majewski@samsung.com>
> 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>
> ---
>  Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt |   11 +++++++++++
>  1 file changed, 11 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..d925e38
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt
> @@ -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.
> 

Won't single entry for boost frequency suffice which would be the starting
frequency in the boost range. IWO will there be OPP list with frequencies:
A > B > C > D, but only B and C are boost frequency. That seems little odd,
unless it's some configuration chosen purely on software basis rather than
hardware. For me B marks the beginning of over-clocking.

Regards,
Sudeep

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
@ 2014-02-07 16:15     ` Sudeep Holla
  0 siblings, 0 replies; 54+ messages in thread
From: Sudeep Holla @ 2014-02-07 16:15 UTC (permalink / raw)
  To: linux-arm-kernel

On 07/02/14 15:19, 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: Nishanth Menon <nm@ti.com>
> Cc: Lukasz Majewski <l.majewski@samsung.com>
> 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>
> ---
>  Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt |   11 +++++++++++
>  1 file changed, 11 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..d925e38
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt
> @@ -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.
> 

Won't single entry for boost frequency suffice which would be the starting
frequency in the boost range. IWO will there be OPP list with frequencies:
A > B > C > D, but only B and C are boost frequency. That seems little odd,
unless it's some configuration chosen purely on software basis rather than
hardware. For me B marks the beginning of over-clocking.

Regards,
Sudeep

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
  2014-02-07 16:15     ` Sudeep Holla
@ 2014-02-07 16:28       ` Sudeep Holla
  -1 siblings, 0 replies; 54+ messages in thread
From: Sudeep Holla @ 2014-02-07 16:28 UTC (permalink / raw)
  To: Thomas Abraham, linux-pm, devicetree, linux-arm-kernel
  Cc: Sudeep Holla, rjw, linux-samsung-soc, kgene.kim, t.figa,
	l.majewski, viresh.kumar, thomas.ab, Nishanth Menon, Rob Herring,
	Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala

On 07/02/14 16:15, Sudeep Holla wrote:
> On 07/02/14 15:19, 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: Nishanth Menon <nm@ti.com>
>> Cc: Lukasz Majewski <l.majewski@samsung.com>
>> 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>
>> ---
>>  Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt |   11 +++++++++++
>>  1 file changed, 11 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..d925e38
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt
>> @@ -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.
>>
> 
> Won't single entry for boost frequency suffice which would be the starting
> frequency in the boost range. IOW will there be OPP list with frequencies:
> A > B > C > D, but only B and C are boost frequency. That seems little odd,
> unless it's some configuration chosen purely on software basis rather than
> hardware. For me B marks the beginning of over-clocking.
> 
Ah, I meant A < B < C < D in the above example.

Regards,
Sudeep

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
@ 2014-02-07 16:28       ` Sudeep Holla
  0 siblings, 0 replies; 54+ messages in thread
From: Sudeep Holla @ 2014-02-07 16:28 UTC (permalink / raw)
  To: linux-arm-kernel

On 07/02/14 16:15, Sudeep Holla wrote:
> On 07/02/14 15:19, 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: Nishanth Menon <nm@ti.com>
>> Cc: Lukasz Majewski <l.majewski@samsung.com>
>> 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>
>> ---
>>  Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt |   11 +++++++++++
>>  1 file changed, 11 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..d925e38
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt
>> @@ -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.
>>
> 
> Won't single entry for boost frequency suffice which would be the starting
> frequency in the boost range. IOW will there be OPP list with frequencies:
> A > B > C > D, but only B and C are boost frequency. That seems little odd,
> unless it's some configuration chosen purely on software basis rather than
> hardware. For me B marks the beginning of over-clocking.
> 
Ah, I meant A < B < C < D in the above example.

Regards,
Sudeep

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
  2014-02-07 16:28       ` Sudeep Holla
@ 2014-02-07 16:43         ` Nishanth Menon
  -1 siblings, 0 replies; 54+ messages in thread
From: Nishanth Menon @ 2014-02-07 16:43 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: Thomas Abraham, linux-pm, devicetree, linux-arm-kernel, rjw,
	linux-samsung-soc, kgene.kim, t.figa, l.majewski, viresh.kumar,
	thomas.ab, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala

On Fri, Feb 7, 2014 at 10:28 AM, Sudeep Holla <Sudeep.Holla@arm.com> wrote:
> On 07/02/14 16:15, Sudeep Holla wrote:
>> On 07/02/14 15:19, 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: Nishanth Menon <nm@ti.com>
>>> Cc: Lukasz Majewski <l.majewski@samsung.com>
>>> 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>
>>> ---
>>>  Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt |   11 +++++++++++
>>>  1 file changed, 11 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..d925e38
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt
>>> @@ -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.
>>>
>>
>> Won't single entry for boost frequency suffice which would be the starting
>> frequency in the boost range. IOW will there be OPP list with frequencies:
>> A > B > C > D, but only B and C are boost frequency. That seems little odd,
>> unless it's some configuration chosen purely on software basis rather than
>> hardware. For me B marks the beginning of over-clocking.
>>
> Ah, I meant A < B < C < D in the above example.
Should'nt we let the SoC dts define that - traditionally, yes, but
consider the following:
A, B, C uses clk_parent X which describes B, C as overclocked.
and say D uses clk_parent Y which is not "over clocked", then you have
the scenario that on the first look seems counter-intutive.

Regards,
Nishanth Menon

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
@ 2014-02-07 16:43         ` Nishanth Menon
  0 siblings, 0 replies; 54+ messages in thread
From: Nishanth Menon @ 2014-02-07 16:43 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Feb 7, 2014 at 10:28 AM, Sudeep Holla <Sudeep.Holla@arm.com> wrote:
> On 07/02/14 16:15, Sudeep Holla wrote:
>> On 07/02/14 15:19, 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: Nishanth Menon <nm@ti.com>
>>> Cc: Lukasz Majewski <l.majewski@samsung.com>
>>> 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>
>>> ---
>>>  Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt |   11 +++++++++++
>>>  1 file changed, 11 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..d925e38
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt
>>> @@ -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.
>>>
>>
>> Won't single entry for boost frequency suffice which would be the starting
>> frequency in the boost range. IOW will there be OPP list with frequencies:
>> A > B > C > D, but only B and C are boost frequency. That seems little odd,
>> unless it's some configuration chosen purely on software basis rather than
>> hardware. For me B marks the beginning of over-clocking.
>>
> Ah, I meant A < B < C < D in the above example.
Should'nt we let the SoC dts define that - traditionally, yes, but
consider the following:
A, B, C uses clk_parent X which describes B, C as overclocked.
and say D uses clk_parent Y which is not "over clocked", then you have
the scenario that on the first look seems counter-intutive.

Regards,
Nishanth Menon

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
  2014-02-07 16:43         ` Nishanth Menon
@ 2014-02-07 17:31           ` Sudeep Holla
  -1 siblings, 0 replies; 54+ messages in thread
From: Sudeep Holla @ 2014-02-07 17:31 UTC (permalink / raw)
  To: Nishanth Menon
  Cc: Sudeep.Holla, Thomas Abraham, linux-pm, devicetree,
	linux-arm-kernel, rjw, linux-samsung-soc, kgene.kim, t.figa,
	l.majewski, viresh.kumar, thomas.ab, Rob Herring, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala

On 07/02/14 16:43, Nishanth Menon wrote:
> On Fri, Feb 7, 2014 at 10:28 AM, Sudeep Holla <Sudeep.Holla@arm.com> wrote:
>> On 07/02/14 16:15, Sudeep Holla wrote:
>>> On 07/02/14 15:19, 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: Nishanth Menon <nm@ti.com>
>>>> Cc: Lukasz Majewski <l.majewski@samsung.com>
>>>> 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>
>>>> ---
>>>>  Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt |   11 +++++++++++
>>>>  1 file changed, 11 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..d925e38
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt
>>>> @@ -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.
>>>>
>>>
>>> Won't single entry for boost frequency suffice which would be the starting
>>> frequency in the boost range. IOW will there be OPP list with frequencies:
>>> A > B > C > D, but only B and C are boost frequency. That seems little odd,
>>> unless it's some configuration chosen purely on software basis rather than
>>> hardware. For me B marks the beginning of over-clocking.
>>>
>> Ah, I meant A < B < C < D in the above example.
>
> Should'nt we let the SoC dts define that - traditionally, yes, but
> consider the following:
> A, B, C uses clk_parent X which describes B, C as overclocked.
> and say D uses clk_parent Y which is not "over clocked", then you have
> the scenario that on the first look seems counter-intutive.
> 

Yes I thought of exactly similar clock setup, but was not convinced that it
should be part of OPP. In that case it looks like we are trying to represent
clock internals through some OPP bindings.

Yes I think its counter-intuitive as it's visible to the userspace(list of
frequencies and the boost parameters are exposed through sysfs)

Regards,
Sudeep



^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
@ 2014-02-07 17:31           ` Sudeep Holla
  0 siblings, 0 replies; 54+ messages in thread
From: Sudeep Holla @ 2014-02-07 17:31 UTC (permalink / raw)
  To: linux-arm-kernel

On 07/02/14 16:43, Nishanth Menon wrote:
> On Fri, Feb 7, 2014 at 10:28 AM, Sudeep Holla <Sudeep.Holla@arm.com> wrote:
>> On 07/02/14 16:15, Sudeep Holla wrote:
>>> On 07/02/14 15:19, 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: Nishanth Menon <nm@ti.com>
>>>> Cc: Lukasz Majewski <l.majewski@samsung.com>
>>>> 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>
>>>> ---
>>>>  Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt |   11 +++++++++++
>>>>  1 file changed, 11 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..d925e38
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt
>>>> @@ -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.
>>>>
>>>
>>> Won't single entry for boost frequency suffice which would be the starting
>>> frequency in the boost range. IOW will there be OPP list with frequencies:
>>> A > B > C > D, but only B and C are boost frequency. That seems little odd,
>>> unless it's some configuration chosen purely on software basis rather than
>>> hardware. For me B marks the beginning of over-clocking.
>>>
>> Ah, I meant A < B < C < D in the above example.
>
> Should'nt we let the SoC dts define that - traditionally, yes, but
> consider the following:
> A, B, C uses clk_parent X which describes B, C as overclocked.
> and say D uses clk_parent Y which is not "over clocked", then you have
> the scenario that on the first look seems counter-intutive.
> 

Yes I thought of exactly similar clock setup, but was not convinced that it
should be part of OPP. In that case it looks like we are trying to represent
clock internals through some OPP bindings.

Yes I think its counter-intuitive as it's visible to the userspace(list of
frequencies and the boost parameters are exposed through sysfs)

Regards,
Sudeep

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
  2014-02-07 17:31           ` Sudeep Holla
@ 2014-02-07 17:37             ` Nishanth Menon
  -1 siblings, 0 replies; 54+ messages in thread
From: Nishanth Menon @ 2014-02-07 17:37 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: Thomas Abraham, linux-pm, devicetree, linux-arm-kernel, rjw,
	linux-samsung-soc, kgene.kim, t.figa, l.majewski, viresh.kumar,
	thomas.ab, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala

On Fri, Feb 7, 2014 at 11:31 AM, Sudeep Holla <Sudeep.Holla@arm.com> wrote:
>
> Yes I thought of exactly similar clock setup, but was not convinced that it
> should be part of OPP. In that case it looks like we are trying to represent
> clock internals through some OPP bindings.

And this series (rightly) does not make it an OPP behavior. instead
all it does is to list the boost-frequencies and mark those in cpufreq
table. the description is left to the dts and implementation to the
clock drivers involved.


> Yes I think its counter-intuitive as it's visible to the userspace(list of
> frequencies and the boost parameters are exposed through sysfs)

That will be a different problem -> as currently every single
frequency in the cpufreq list has ability to be marked as boost
frequency - if userspace does not maintain that, then, IMHO, fix the
userspace :D

Regards,
Nishanth Menon

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
@ 2014-02-07 17:37             ` Nishanth Menon
  0 siblings, 0 replies; 54+ messages in thread
From: Nishanth Menon @ 2014-02-07 17:37 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Feb 7, 2014 at 11:31 AM, Sudeep Holla <Sudeep.Holla@arm.com> wrote:
>
> Yes I thought of exactly similar clock setup, but was not convinced that it
> should be part of OPP. In that case it looks like we are trying to represent
> clock internals through some OPP bindings.

And this series (rightly) does not make it an OPP behavior. instead
all it does is to list the boost-frequencies and mark those in cpufreq
table. the description is left to the dts and implementation to the
clock drivers involved.


> Yes I think its counter-intuitive as it's visible to the userspace(list of
> frequencies and the boost parameters are exposed through sysfs)

That will be a different problem -> as currently every single
frequency in the cpufreq list has ability to be marked as boost
frequency - if userspace does not maintain that, then, IMHO, fix the
userspace :D

Regards,
Nishanth Menon

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
  2014-02-07 17:37             ` Nishanth Menon
@ 2014-02-07 17:40               ` Nishanth Menon
  -1 siblings, 0 replies; 54+ messages in thread
From: Nishanth Menon @ 2014-02-07 17:40 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: Thomas Abraham, linux-pm, devicetree, linux-arm-kernel, rjw,
	linux-samsung-soc, kgene.kim, t.figa, l.majewski, viresh.kumar,
	thomas.ab, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala

On Fri, Feb 7, 2014 at 11:37 AM, Nishanth Menon <nm@ti.com> wrote:
> On Fri, Feb 7, 2014 at 11:31 AM, Sudeep Holla <Sudeep.Holla@arm.com> wrote:
>>
>> Yes I thought of exactly similar clock setup, but was not convinced that it
>> should be part of OPP. In that case it looks like we are trying to represent
>> clock internals through some OPP bindings.
>
> And this series (rightly) does not make it an OPP behavior. instead
> all it does is to list the boost-frequencies and mark those in cpufreq
> table. the description is left to the dts and implementation to the
> clock drivers involved.


One more thing, before I forget -> currently
dev_pm_opp_[init|free]_cpufreq_table is in drivers/base/power/opp.c ->
this probably should go away to drivers/cpufreq to keep opp.c
independent of frameworks using it. i dont see any code that is
introduced in the mentioned functions as being OPP behavior specific,
instead, I consider them as cpufreq+opp behaviors, which this change
fits into.

Regards,
Nishanth Menon

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
@ 2014-02-07 17:40               ` Nishanth Menon
  0 siblings, 0 replies; 54+ messages in thread
From: Nishanth Menon @ 2014-02-07 17:40 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Feb 7, 2014 at 11:37 AM, Nishanth Menon <nm@ti.com> wrote:
> On Fri, Feb 7, 2014 at 11:31 AM, Sudeep Holla <Sudeep.Holla@arm.com> wrote:
>>
>> Yes I thought of exactly similar clock setup, but was not convinced that it
>> should be part of OPP. In that case it looks like we are trying to represent
>> clock internals through some OPP bindings.
>
> And this series (rightly) does not make it an OPP behavior. instead
> all it does is to list the boost-frequencies and mark those in cpufreq
> table. the description is left to the dts and implementation to the
> clock drivers involved.


One more thing, before I forget -> currently
dev_pm_opp_[init|free]_cpufreq_table is in drivers/base/power/opp.c ->
this probably should go away to drivers/cpufreq to keep opp.c
independent of frameworks using it. i dont see any code that is
introduced in the mentioned functions as being OPP behavior specific,
instead, I consider them as cpufreq+opp behaviors, which this change
fits into.

Regards,
Nishanth Menon

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
  2014-02-07 17:37             ` Nishanth Menon
@ 2014-02-07 18:02               ` Sudeep Holla
  -1 siblings, 0 replies; 54+ messages in thread
From: Sudeep Holla @ 2014-02-07 18:02 UTC (permalink / raw)
  To: Nishanth Menon
  Cc: Sudeep.Holla, Thomas Abraham, linux-pm, devicetree,
	linux-arm-kernel, rjw, linux-samsung-soc, kgene.kim, t.figa,
	l.majewski, viresh.kumar, thomas.ab, Rob Herring, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala

On 07/02/14 17:37, Nishanth Menon wrote:
> On Fri, Feb 7, 2014 at 11:31 AM, Sudeep Holla <Sudeep.Holla@arm.com> wrote:

[...]

>> Yes I think its counter-intuitive as it's visible to the userspace(list of
>> frequencies and the boost parameters are exposed through sysfs)
> 
> That will be a different problem -> as currently every single
> frequency in the cpufreq list has ability to be marked as boost
> frequency - if userspace does not maintain that, then, IMHO, fix the
> userspace :D
> 

/sys/devices/system/cpu/cpu*/cpufreq/scaling_available_frequencies gives
the list of frequencies based on the state of the boost feature at anytime.

Intuitively the list without boost shouldn't have any frequency above the range
when it's enabled :), that's what I was referring to. So I am not talking about
any issue with user-space maintenance.

Regards,
Sudeep

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
@ 2014-02-07 18:02               ` Sudeep Holla
  0 siblings, 0 replies; 54+ messages in thread
From: Sudeep Holla @ 2014-02-07 18:02 UTC (permalink / raw)
  To: linux-arm-kernel

On 07/02/14 17:37, Nishanth Menon wrote:
> On Fri, Feb 7, 2014 at 11:31 AM, Sudeep Holla <Sudeep.Holla@arm.com> wrote:

[...]

>> Yes I think its counter-intuitive as it's visible to the userspace(list of
>> frequencies and the boost parameters are exposed through sysfs)
> 
> That will be a different problem -> as currently every single
> frequency in the cpufreq list has ability to be marked as boost
> frequency - if userspace does not maintain that, then, IMHO, fix the
> userspace :D
> 

/sys/devices/system/cpu/cpu*/cpufreq/scaling_available_frequencies gives
the list of frequencies based on the state of the boost feature at anytime.

Intuitively the list without boost shouldn't have any frequency above the range
when it's enabled :), that's what I was referring to. So I am not talking about
any issue with user-space maintenance.

Regards,
Sudeep

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
  2014-02-07 18:02               ` Sudeep Holla
@ 2014-02-07 19:41                 ` Nishanth Menon
  -1 siblings, 0 replies; 54+ messages in thread
From: Nishanth Menon @ 2014-02-07 19:41 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: Thomas Abraham, linux-pm, devicetree, linux-arm-kernel, rjw,
	linux-samsung-soc, kgene.kim, t.figa, l.majewski, viresh.kumar,
	thomas.ab, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala

On Fri, Feb 7, 2014 at 12:02 PM, Sudeep Holla <Sudeep.Holla@arm.com> wrote:
> On 07/02/14 17:37, Nishanth Menon wrote:
>> On Fri, Feb 7, 2014 at 11:31 AM, Sudeep Holla <Sudeep.Holla@arm.com> wrote:
>
> [...]
>
>>> Yes I think its counter-intuitive as it's visible to the userspace(list of
>>> frequencies and the boost parameters are exposed through sysfs)
>>
>> That will be a different problem -> as currently every single
>> frequency in the cpufreq list has ability to be marked as boost
>> frequency - if userspace does not maintain that, then, IMHO, fix the
>> userspace :D
>>
>
> /sys/devices/system/cpu/cpu*/cpufreq/scaling_available_frequencies gives
> the list of frequencies based on the state of the boost feature at anytime.
>
> Intuitively the list without boost shouldn't have any frequency above the range
> when it's enabled :), that's what I was referring to. So I am not talking about
> any issue with user-space maintenance.
Fair enough - but i still think it has nothing to do with dt binding
itself -> and i think the discussion we've had should be good for the
binding provided in this patch.. I hope.. if documentation needs a bit
of better explanation to prevent a repeat of the same discussion at a
later point of time, now might be a good time to add it in.

Regards,
Nishanth Menon

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
@ 2014-02-07 19:41                 ` Nishanth Menon
  0 siblings, 0 replies; 54+ messages in thread
From: Nishanth Menon @ 2014-02-07 19:41 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Feb 7, 2014 at 12:02 PM, Sudeep Holla <Sudeep.Holla@arm.com> wrote:
> On 07/02/14 17:37, Nishanth Menon wrote:
>> On Fri, Feb 7, 2014 at 11:31 AM, Sudeep Holla <Sudeep.Holla@arm.com> wrote:
>
> [...]
>
>>> Yes I think its counter-intuitive as it's visible to the userspace(list of
>>> frequencies and the boost parameters are exposed through sysfs)
>>
>> That will be a different problem -> as currently every single
>> frequency in the cpufreq list has ability to be marked as boost
>> frequency - if userspace does not maintain that, then, IMHO, fix the
>> userspace :D
>>
>
> /sys/devices/system/cpu/cpu*/cpufreq/scaling_available_frequencies gives
> the list of frequencies based on the state of the boost feature at anytime.
>
> Intuitively the list without boost shouldn't have any frequency above the range
> when it's enabled :), that's what I was referring to. So I am not talking about
> any issue with user-space maintenance.
Fair enough - but i still think it has nothing to do with dt binding
itself -> and i think the discussion we've had should be good for the
binding provided in this patch.. I hope.. if documentation needs a bit
of better explanation to prevent a repeat of the same discussion at a
later point of time, now might be a good time to add it in.

Regards,
Nishanth Menon

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v2 1/2] PM / OPP: Allow boost frequency to be looked up from device tree
  2014-02-07 16:01     ` Sudeep Holla
@ 2014-02-08  5:10       ` Thomas Abraham
  -1 siblings, 0 replies; 54+ messages in thread
From: Thomas Abraham @ 2014-02-08  5:10 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: linux-pm, devicetree, linux-arm-kernel, rjw, linux-samsung-soc,
	kgene.kim, t.figa, l.majewski, viresh.kumar, thomas.ab,
	Nishanth Menon

On Fri, Feb 7, 2014 at 9:31 PM, Sudeep Holla <Sudeep.Holla@arm.com> wrote:
> On 07/02/14 15:19, Thomas Abraham wrote:
>> From: Thomas Abraham <thomas.ab@samsung.com>
>>
>> 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 <nm@ti.com>
>> Cc: Lukasz Majewski <l.majewski@samsung.com>
>> Signed-off-by: Thomas Abraham <thomas.ab@samsung.com>
>> ---
>>  drivers/base/power/opp.c |   34 +++++++++++++++++++++++++++++++++-
>>  1 file changed, 33 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
>> index fa41874..b636826 100644
>> --- a/drivers/base/power/opp.c
>> +++ b/drivers/base/power/opp.c
>> @@ -628,7 +628,8 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
>>       struct device_opp *dev_opp;
>>       struct dev_pm_opp *opp;
>>       struct cpufreq_frequency_table *freq_table;
>> -     int i = 0;
>> +     int i = 0, j, len, ret;
>> +     u32 *boost_freqs = NULL;
>>
>>       /* Pretend as if I am an updater */
>>       mutex_lock(&dev_opp_list_lock);
>> @@ -650,10 +651,35 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
>>               return -ENOMEM;
>>       }
>>
>> +     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 err_boost;
>> +             }
>> +
>> +             boost_freqs = kzalloc(len, GFP_KERNEL);
>> +             if (!boost_freqs) {
>> +                     dev_warn(dev, "%s: no memory for boost freq table\n",
>> +                                     __func__);
>> +                     ret = -ENOMEM;
>> +                     goto err_boost;
>> +             }
>> +             of_property_read_u32_array(dev->of_node, "boost-frequency",
>> +                     boost_freqs, len / sizeof(u32));
>> +     }
>> +
>>       list_for_each_entry(opp, &dev_opp->opp_list, node) {
>>               if (opp->available) {
>>                       freq_table[i].driver_data = i;
>>                       freq_table[i].frequency = opp->rate / 1000;
>> +                     for (j = 0; j < len / sizeof(u32) && boost_freqs; j++) {
>> +                             if (boost_freqs[j] == freq_table[i].frequency) {
>> +                                     freq_table[i].driver_data =
>> +                                                     CPUFREQ_BOOST_FREQ;
>> +                                     break;
>> +                             }
>> +                     }
>>                       i++;
>>               }
>>       }
> IIRC you had mentioned that the boost-opp was not limited to be a cpufreq, but
> this change seems to be cpufreq only.

Yes, but as you have initiated the discussion on extending the OPP
binding, this has been limited to cpufreq only. If the new OPP library
has support for listing boost frequency, this can be migrated to the
new OPP libaray.

Thanks,
Thomas.

>
> Regards,
> Sudeep
>

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v2 1/2] PM / OPP: Allow boost frequency to be looked up from device tree
@ 2014-02-08  5:10       ` Thomas Abraham
  0 siblings, 0 replies; 54+ messages in thread
From: Thomas Abraham @ 2014-02-08  5:10 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Feb 7, 2014 at 9:31 PM, Sudeep Holla <Sudeep.Holla@arm.com> wrote:
> On 07/02/14 15:19, Thomas Abraham wrote:
>> From: Thomas Abraham <thomas.ab@samsung.com>
>>
>> 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 <nm@ti.com>
>> Cc: Lukasz Majewski <l.majewski@samsung.com>
>> Signed-off-by: Thomas Abraham <thomas.ab@samsung.com>
>> ---
>>  drivers/base/power/opp.c |   34 +++++++++++++++++++++++++++++++++-
>>  1 file changed, 33 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
>> index fa41874..b636826 100644
>> --- a/drivers/base/power/opp.c
>> +++ b/drivers/base/power/opp.c
>> @@ -628,7 +628,8 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
>>       struct device_opp *dev_opp;
>>       struct dev_pm_opp *opp;
>>       struct cpufreq_frequency_table *freq_table;
>> -     int i = 0;
>> +     int i = 0, j, len, ret;
>> +     u32 *boost_freqs = NULL;
>>
>>       /* Pretend as if I am an updater */
>>       mutex_lock(&dev_opp_list_lock);
>> @@ -650,10 +651,35 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
>>               return -ENOMEM;
>>       }
>>
>> +     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 err_boost;
>> +             }
>> +
>> +             boost_freqs = kzalloc(len, GFP_KERNEL);
>> +             if (!boost_freqs) {
>> +                     dev_warn(dev, "%s: no memory for boost freq table\n",
>> +                                     __func__);
>> +                     ret = -ENOMEM;
>> +                     goto err_boost;
>> +             }
>> +             of_property_read_u32_array(dev->of_node, "boost-frequency",
>> +                     boost_freqs, len / sizeof(u32));
>> +     }
>> +
>>       list_for_each_entry(opp, &dev_opp->opp_list, node) {
>>               if (opp->available) {
>>                       freq_table[i].driver_data = i;
>>                       freq_table[i].frequency = opp->rate / 1000;
>> +                     for (j = 0; j < len / sizeof(u32) && boost_freqs; j++) {
>> +                             if (boost_freqs[j] == freq_table[i].frequency) {
>> +                                     freq_table[i].driver_data =
>> +                                                     CPUFREQ_BOOST_FREQ;
>> +                                     break;
>> +                             }
>> +                     }
>>                       i++;
>>               }
>>       }
> IIRC you had mentioned that the boost-opp was not limited to be a cpufreq, but
> this change seems to be cpufreq only.

Yes, but as you have initiated the discussion on extending the OPP
binding, this has been limited to cpufreq only. If the new OPP library
has support for listing boost frequency, this can be migrated to the
new OPP libaray.

Thanks,
Thomas.

>
> Regards,
> Sudeep
>

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
  2014-02-07 18:02               ` Sudeep Holla
@ 2014-02-08  6:47                 ` Thomas Abraham
  -1 siblings, 0 replies; 54+ messages in thread
From: Thomas Abraham @ 2014-02-08  6:47 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: Nishanth Menon, linux-pm, devicetree, linux-arm-kernel, rjw,
	linux-samsung-soc, kgene.kim, t.figa, l.majewski, viresh.kumar,
	thomas.ab, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala

On Fri, Feb 7, 2014 at 11:32 PM, Sudeep Holla <Sudeep.Holla@arm.com> wrote:
> On 07/02/14 17:37, Nishanth Menon wrote:
>> On Fri, Feb 7, 2014 at 11:31 AM, Sudeep Holla <Sudeep.Holla@arm.com> wrote:
>
> [...]
>
>>> Yes I think its counter-intuitive as it's visible to the userspace(list of
>>> frequencies and the boost parameters are exposed through sysfs)
>>
>> That will be a different problem -> as currently every single
>> frequency in the cpufreq list has ability to be marked as boost
>> frequency - if userspace does not maintain that, then, IMHO, fix the
>> userspace :D
>>
>
> /sys/devices/system/cpu/cpu*/cpufreq/scaling_available_frequencies gives
> the list of frequencies based on the state of the boost feature at anytime.

The list of frequencies in
/sys/devices/system/cpu/cpu*/cpufreq/scaling_available_frequencies
does not change based in the state of the boost feature (enabled or
disabled). But the scaling_max_frequency and scaling_min_frequency are
updated based on the set of available + boost frequencies available.

>
> Intuitively the list without boost shouldn't have any frequency above the range
> when it's enabled :), that's what I was referring to. So I am not talking about
> any issue with user-space maintenance.
>
> Regards,
> Sudeep
>

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
@ 2014-02-08  6:47                 ` Thomas Abraham
  0 siblings, 0 replies; 54+ messages in thread
From: Thomas Abraham @ 2014-02-08  6:47 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Feb 7, 2014 at 11:32 PM, Sudeep Holla <Sudeep.Holla@arm.com> wrote:
> On 07/02/14 17:37, Nishanth Menon wrote:
>> On Fri, Feb 7, 2014 at 11:31 AM, Sudeep Holla <Sudeep.Holla@arm.com> wrote:
>
> [...]
>
>>> Yes I think its counter-intuitive as it's visible to the userspace(list of
>>> frequencies and the boost parameters are exposed through sysfs)
>>
>> That will be a different problem -> as currently every single
>> frequency in the cpufreq list has ability to be marked as boost
>> frequency - if userspace does not maintain that, then, IMHO, fix the
>> userspace :D
>>
>
> /sys/devices/system/cpu/cpu*/cpufreq/scaling_available_frequencies gives
> the list of frequencies based on the state of the boost feature at anytime.

The list of frequencies in
/sys/devices/system/cpu/cpu*/cpufreq/scaling_available_frequencies
does not change based in the state of the boost feature (enabled or
disabled). But the scaling_max_frequency and scaling_min_frequency are
updated based on the set of available + boost frequencies available.

>
> Intuitively the list without boost shouldn't have any frequency above the range
> when it's enabled :), that's what I was referring to. So I am not talking about
> any issue with user-space maintenance.
>
> Regards,
> Sudeep
>

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
  2014-02-07 19:41                 ` Nishanth Menon
@ 2014-02-08  6:47                   ` Thomas Abraham
  -1 siblings, 0 replies; 54+ messages in thread
From: Thomas Abraham @ 2014-02-08  6:47 UTC (permalink / raw)
  To: Nishanth Menon
  Cc: Sudeep Holla, linux-pm, devicetree, linux-arm-kernel, rjw,
	linux-samsung-soc, kgene.kim, t.figa, l.majewski, viresh.kumar,
	thomas.ab, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala

On Sat, Feb 8, 2014 at 1:11 AM, Nishanth Menon <nm@ti.com> wrote:
> On Fri, Feb 7, 2014 at 12:02 PM, Sudeep Holla <Sudeep.Holla@arm.com> wrote:
>> On 07/02/14 17:37, Nishanth Menon wrote:
>>> On Fri, Feb 7, 2014 at 11:31 AM, Sudeep Holla <Sudeep.Holla@arm.com> wrote:
>>
>> [...]
>>
>>>> Yes I think its counter-intuitive as it's visible to the userspace(list of
>>>> frequencies and the boost parameters are exposed through sysfs)
>>>
>>> That will be a different problem -> as currently every single
>>> frequency in the cpufreq list has ability to be marked as boost
>>> frequency - if userspace does not maintain that, then, IMHO, fix the
>>> userspace :D
>>>
>>
>> /sys/devices/system/cpu/cpu*/cpufreq/scaling_available_frequencies gives
>> the list of frequencies based on the state of the boost feature at anytime.
>>
>> Intuitively the list without boost shouldn't have any frequency above the range
>> when it's enabled :), that's what I was referring to. So I am not talking about
>> any issue with user-space maintenance.
> Fair enough - but i still think it has nothing to do with dt binding
> itself -> and i think the discussion we've had should be good for the
> binding provided in this patch.. I hope.. if documentation needs a bit
> of better explanation to prevent a repeat of the same discussion at a
> later point of time, now might be a good time to add it in.

The term boost and over-clocking have been described in the bindings
document as being the same. Since the term over-clocking refers to
running a CPU beyond normal operating frequencies, I tend to agree
with Sudeep that it is not intuitive if a normal operating frequency
is greater than a boost mode frequency.

Otherwise, when userspace does "echo 1 >
/sys/devices/system/cpu/cpufreq/boost", what is it supposed to mean. I
think the original intent of boost mode patches was to allow CPU to
operate at frequencies greater than the normal operating frequencies.

Lukasz, how would you want this to be handled?

Thanks,
Thomas.

>
> Regards,
> Nishanth Menon

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
@ 2014-02-08  6:47                   ` Thomas Abraham
  0 siblings, 0 replies; 54+ messages in thread
From: Thomas Abraham @ 2014-02-08  6:47 UTC (permalink / raw)
  To: linux-arm-kernel

On Sat, Feb 8, 2014 at 1:11 AM, Nishanth Menon <nm@ti.com> wrote:
> On Fri, Feb 7, 2014 at 12:02 PM, Sudeep Holla <Sudeep.Holla@arm.com> wrote:
>> On 07/02/14 17:37, Nishanth Menon wrote:
>>> On Fri, Feb 7, 2014 at 11:31 AM, Sudeep Holla <Sudeep.Holla@arm.com> wrote:
>>
>> [...]
>>
>>>> Yes I think its counter-intuitive as it's visible to the userspace(list of
>>>> frequencies and the boost parameters are exposed through sysfs)
>>>
>>> That will be a different problem -> as currently every single
>>> frequency in the cpufreq list has ability to be marked as boost
>>> frequency - if userspace does not maintain that, then, IMHO, fix the
>>> userspace :D
>>>
>>
>> /sys/devices/system/cpu/cpu*/cpufreq/scaling_available_frequencies gives
>> the list of frequencies based on the state of the boost feature at anytime.
>>
>> Intuitively the list without boost shouldn't have any frequency above the range
>> when it's enabled :), that's what I was referring to. So I am not talking about
>> any issue with user-space maintenance.
> Fair enough - but i still think it has nothing to do with dt binding
> itself -> and i think the discussion we've had should be good for the
> binding provided in this patch.. I hope.. if documentation needs a bit
> of better explanation to prevent a repeat of the same discussion at a
> later point of time, now might be a good time to add it in.

The term boost and over-clocking have been described in the bindings
document as being the same. Since the term over-clocking refers to
running a CPU beyond normal operating frequencies, I tend to agree
with Sudeep that it is not intuitive if a normal operating frequency
is greater than a boost mode frequency.

Otherwise, when userspace does "echo 1 >
/sys/devices/system/cpu/cpufreq/boost", what is it supposed to mean. I
think the original intent of boost mode patches was to allow CPU to
operate at frequencies greater than the normal operating frequencies.

Lukasz, how would you want this to be handled?

Thanks,
Thomas.

>
> Regards,
> Nishanth Menon

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v2 1/2] PM / OPP: Allow boost frequency to be looked up from device tree
  2014-02-08  5:10       ` Thomas Abraham
@ 2014-02-10  7:10         ` Lukasz Majewski
  -1 siblings, 0 replies; 54+ messages in thread
From: Lukasz Majewski @ 2014-02-10  7:10 UTC (permalink / raw)
  To: Thomas Abraham, Sudeep Holla
  Cc: Nishanth Menon, devicetree, linux-samsung-soc, linux-pm,
	viresh.kumar, t.figa, rjw, kgene.kim, thomas.ab,
	linux-arm-kernel

Hi Thomas, Sudeep

> On Fri, Feb 7, 2014 at 9:31 PM, Sudeep Holla <Sudeep.Holla@arm.com>
> wrote:
> > On 07/02/14 15:19, Thomas Abraham wrote:
> >> From: Thomas Abraham <thomas.ab@samsung.com>
> >>
> >> 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 <nm@ti.com>
> >> Cc: Lukasz Majewski <l.majewski@samsung.com>
> >> Signed-off-by: Thomas Abraham <thomas.ab@samsung.com>
> >> ---
> >>  drivers/base/power/opp.c |   34 +++++++++++++++++++++++++++++++++-
> >>  1 file changed, 33 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
> >> index fa41874..b636826 100644
> >> --- a/drivers/base/power/opp.c
> >> +++ b/drivers/base/power/opp.c
> >> @@ -628,7 +628,8 @@ int dev_pm_opp_init_cpufreq_table(struct
> >> device *dev, struct device_opp *dev_opp;
> >>       struct dev_pm_opp *opp;
> >>       struct cpufreq_frequency_table *freq_table;
> >> -     int i = 0;
> >> +     int i = 0, j, len, ret;
> >> +     u32 *boost_freqs = NULL;
> >>
> >>       /* Pretend as if I am an updater */
> >>       mutex_lock(&dev_opp_list_lock);
> >> @@ -650,10 +651,35 @@ int dev_pm_opp_init_cpufreq_table(struct
> >> device *dev, return -ENOMEM;
> >>       }
> >>
> >> +     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 err_boost;
> >> +             }
> >> +
> >> +             boost_freqs = kzalloc(len, GFP_KERNEL);
> >> +             if (!boost_freqs) {
> >> +                     dev_warn(dev, "%s: no memory for boost freq
> >> table\n",
> >> +                                     __func__);
> >> +                     ret = -ENOMEM;
> >> +                     goto err_boost;
> >> +             }
> >> +             of_property_read_u32_array(dev->of_node,
> >> "boost-frequency",
> >> +                     boost_freqs, len / sizeof(u32));
> >> +     }
> >> +
> >>       list_for_each_entry(opp, &dev_opp->opp_list, node) {
> >>               if (opp->available) {
> >>                       freq_table[i].driver_data = i;
> >>                       freq_table[i].frequency = opp->rate / 1000;
> >> +                     for (j = 0; j < len / sizeof(u32) &&
> >> boost_freqs; j++) {
> >> +                             if (boost_freqs[j] ==
> >> freq_table[i].frequency) {
> >> +                                     freq_table[i].driver_data =
> >> +
> >> CPUFREQ_BOOST_FREQ;
> >> +                                     break;
> >> +                             }
> >> +                     }
> >>                       i++;
> >>               }
> >>       }
> > IIRC you had mentioned that the boost-opp was not limited to be a
> > cpufreq, but this change seems to be cpufreq only.
> 
> Yes, but as you have initiated the discussion on extending the OPP
> binding, this has been limited to cpufreq only. If the new OPP library
> has support for listing boost frequency, this can be migrated to the
> new OPP libaray.

I agree here with Thomas. Since we are fixing regression here, we shall
use the existing OPP API. When agreement for the future OPPs format is
in place, we will port the boost OPPs to it.

> 
> Thanks,
> Thomas.
> 
> >
> > Regards,
> > Sudeep
> >
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel



-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v2 1/2] PM / OPP: Allow boost frequency to be looked up from device tree
@ 2014-02-10  7:10         ` Lukasz Majewski
  0 siblings, 0 replies; 54+ messages in thread
From: Lukasz Majewski @ 2014-02-10  7:10 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Thomas, Sudeep

> On Fri, Feb 7, 2014 at 9:31 PM, Sudeep Holla <Sudeep.Holla@arm.com>
> wrote:
> > On 07/02/14 15:19, Thomas Abraham wrote:
> >> From: Thomas Abraham <thomas.ab@samsung.com>
> >>
> >> 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 <nm@ti.com>
> >> Cc: Lukasz Majewski <l.majewski@samsung.com>
> >> Signed-off-by: Thomas Abraham <thomas.ab@samsung.com>
> >> ---
> >>  drivers/base/power/opp.c |   34 +++++++++++++++++++++++++++++++++-
> >>  1 file changed, 33 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
> >> index fa41874..b636826 100644
> >> --- a/drivers/base/power/opp.c
> >> +++ b/drivers/base/power/opp.c
> >> @@ -628,7 +628,8 @@ int dev_pm_opp_init_cpufreq_table(struct
> >> device *dev, struct device_opp *dev_opp;
> >>       struct dev_pm_opp *opp;
> >>       struct cpufreq_frequency_table *freq_table;
> >> -     int i = 0;
> >> +     int i = 0, j, len, ret;
> >> +     u32 *boost_freqs = NULL;
> >>
> >>       /* Pretend as if I am an updater */
> >>       mutex_lock(&dev_opp_list_lock);
> >> @@ -650,10 +651,35 @@ int dev_pm_opp_init_cpufreq_table(struct
> >> device *dev, return -ENOMEM;
> >>       }
> >>
> >> +     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 err_boost;
> >> +             }
> >> +
> >> +             boost_freqs = kzalloc(len, GFP_KERNEL);
> >> +             if (!boost_freqs) {
> >> +                     dev_warn(dev, "%s: no memory for boost freq
> >> table\n",
> >> +                                     __func__);
> >> +                     ret = -ENOMEM;
> >> +                     goto err_boost;
> >> +             }
> >> +             of_property_read_u32_array(dev->of_node,
> >> "boost-frequency",
> >> +                     boost_freqs, len / sizeof(u32));
> >> +     }
> >> +
> >>       list_for_each_entry(opp, &dev_opp->opp_list, node) {
> >>               if (opp->available) {
> >>                       freq_table[i].driver_data = i;
> >>                       freq_table[i].frequency = opp->rate / 1000;
> >> +                     for (j = 0; j < len / sizeof(u32) &&
> >> boost_freqs; j++) {
> >> +                             if (boost_freqs[j] ==
> >> freq_table[i].frequency) {
> >> +                                     freq_table[i].driver_data =
> >> +
> >> CPUFREQ_BOOST_FREQ;
> >> +                                     break;
> >> +                             }
> >> +                     }
> >>                       i++;
> >>               }
> >>       }
> > IIRC you had mentioned that the boost-opp was not limited to be a
> > cpufreq, but this change seems to be cpufreq only.
> 
> Yes, but as you have initiated the discussion on extending the OPP
> binding, this has been limited to cpufreq only. If the new OPP library
> has support for listing boost frequency, this can be migrated to the
> new OPP libaray.

I agree here with Thomas. Since we are fixing regression here, we shall
use the existing OPP API. When agreement for the future OPPs format is
in place, we will port the boost OPPs to it.

> 
> Thanks,
> Thomas.
> 
> >
> > Regards,
> > Sudeep
> >
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel



-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
  2014-02-08  6:47                 ` Thomas Abraham
@ 2014-02-10  7:38                   ` Lukasz Majewski
  -1 siblings, 0 replies; 54+ messages in thread
From: Lukasz Majewski @ 2014-02-10  7:38 UTC (permalink / raw)
  To: Thomas Abraham, Sudeep Holla
  Cc: Nishanth Menon, linux-pm, devicetree, linux-arm-kernel, rjw,
	linux-samsung-soc, kgene.kim, t.figa, viresh.kumar, thomas.ab,
	Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala

Hi Thomas, Sudeep,

> On Fri, Feb 7, 2014 at 11:32 PM, Sudeep Holla <Sudeep.Holla@arm.com>
> wrote:
> > On 07/02/14 17:37, Nishanth Menon wrote:
> >> On Fri, Feb 7, 2014 at 11:31 AM, Sudeep Holla
> >> <Sudeep.Holla@arm.com> wrote:
> >
> > [...]
> >
> >>> Yes I think its counter-intuitive as it's visible to the
> >>> userspace(list of frequencies and the boost parameters are
> >>> exposed through sysfs)
> >>
> >> That will be a different problem -> as currently every single
> >> frequency in the cpufreq list has ability to be marked as boost
> >> frequency - if userspace does not maintain that, then, IMHO, fix
> >> the userspace :D
> >>
> >
> > /sys/devices/system/cpu/cpu*/cpufreq/scaling_available_frequencies
> > gives the list of frequencies based on the state of the boost
> > feature at anytime.
> 
> The list of frequencies in
> /sys/devices/system/cpu/cpu*/cpufreq/scaling_available_frequencies
> does not change based in the state of the boost feature (enabled or
> disabled). But the scaling_max_frequency and scaling_min_frequency are
> updated based on the set of available + boost frequencies available.

With boost intended behavior is as follow:

/sys/devices/system/cpu/cpu*/cpufreq/scaling_available_frequencies [1]

shows the non boost frequencies no matter if boost is enabled or not.
Those are the "normal" frequencies.

When boost is supported (by enabling the CONFIG_CPU_FREQ_BOOST_SW)
extra sysfs attribute shows up:

/sys/devices/system/cpu/cpu0/cpufreq/scaling_boost_frequencies [2]
in which are listed only the boost frequencies.

If the dts doesn't provide any values or as it is now no
CPUFREQ_BOOST_FREQ flag is specified (at exynos4x12-cpufreq.c), then
this list is empty.


When boost is disabled the max freq (cpuinfo_max_freq) is in the range
listed at [1].

When boost is enabled this value is the sum of freqs represented at [1]
and [2].

By adding an extra attribute [2] we:
1. Indicate that boost is supported
2. List provided BOOST frequencies
3. Separate boost and non boost freqs


> 
> >
> > Intuitively the list without boost shouldn't have any frequency
> > above the range when it's enabled :), that's what I was referring
> > to. So I am not talking about any issue with user-space maintenance.
> >
> > Regards,
> > Sudeep
> >



-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
@ 2014-02-10  7:38                   ` Lukasz Majewski
  0 siblings, 0 replies; 54+ messages in thread
From: Lukasz Majewski @ 2014-02-10  7:38 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Thomas, Sudeep,

> On Fri, Feb 7, 2014 at 11:32 PM, Sudeep Holla <Sudeep.Holla@arm.com>
> wrote:
> > On 07/02/14 17:37, Nishanth Menon wrote:
> >> On Fri, Feb 7, 2014 at 11:31 AM, Sudeep Holla
> >> <Sudeep.Holla@arm.com> wrote:
> >
> > [...]
> >
> >>> Yes I think its counter-intuitive as it's visible to the
> >>> userspace(list of frequencies and the boost parameters are
> >>> exposed through sysfs)
> >>
> >> That will be a different problem -> as currently every single
> >> frequency in the cpufreq list has ability to be marked as boost
> >> frequency - if userspace does not maintain that, then, IMHO, fix
> >> the userspace :D
> >>
> >
> > /sys/devices/system/cpu/cpu*/cpufreq/scaling_available_frequencies
> > gives the list of frequencies based on the state of the boost
> > feature at anytime.
> 
> The list of frequencies in
> /sys/devices/system/cpu/cpu*/cpufreq/scaling_available_frequencies
> does not change based in the state of the boost feature (enabled or
> disabled). But the scaling_max_frequency and scaling_min_frequency are
> updated based on the set of available + boost frequencies available.

With boost intended behavior is as follow:

/sys/devices/system/cpu/cpu*/cpufreq/scaling_available_frequencies [1]

shows the non boost frequencies no matter if boost is enabled or not.
Those are the "normal" frequencies.

When boost is supported (by enabling the CONFIG_CPU_FREQ_BOOST_SW)
extra sysfs attribute shows up:

/sys/devices/system/cpu/cpu0/cpufreq/scaling_boost_frequencies [2]
in which are listed only the boost frequencies.

If the dts doesn't provide any values or as it is now no
CPUFREQ_BOOST_FREQ flag is specified (at exynos4x12-cpufreq.c), then
this list is empty.


When boost is disabled the max freq (cpuinfo_max_freq) is in the range
listed at [1].

When boost is enabled this value is the sum of freqs represented at [1]
and [2].

By adding an extra attribute [2] we:
1. Indicate that boost is supported
2. List provided BOOST frequencies
3. Separate boost and non boost freqs


> 
> >
> > Intuitively the list without boost shouldn't have any frequency
> > above the range when it's enabled :), that's what I was referring
> > to. So I am not talking about any issue with user-space maintenance.
> >
> > Regards,
> > Sudeep
> >



-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
  2014-02-08  6:47                   ` Thomas Abraham
@ 2014-02-10  7:53                     ` Lukasz Majewski
  -1 siblings, 0 replies; 54+ messages in thread
From: Lukasz Majewski @ 2014-02-10  7:53 UTC (permalink / raw)
  To: Thomas Abraham
  Cc: Nishanth Menon, Mark Rutland, devicetree, linux-samsung-soc,
	Pawel Moll, linux-pm, viresh.kumar, t.figa, Ian Campbell, rjw,
	Rob Herring, kgene.kim, thomas.ab, Sudeep Holla, Kumar Gala,
	linux-arm-kernel

Hi Thomas, Sudeep,

> On Sat, Feb 8, 2014 at 1:11 AM, Nishanth Menon <nm@ti.com> wrote:
> > On Fri, Feb 7, 2014 at 12:02 PM, Sudeep Holla
> > <Sudeep.Holla@arm.com> wrote:
> >> On 07/02/14 17:37, Nishanth Menon wrote:
> >>> On Fri, Feb 7, 2014 at 11:31 AM, Sudeep Holla
> >>> <Sudeep.Holla@arm.com> wrote:
> >>
> >> [...]
> >>
> >>>> Yes I think its counter-intuitive as it's visible to the
> >>>> userspace(list of frequencies and the boost parameters are
> >>>> exposed through sysfs)
> >>>
> >>> That will be a different problem -> as currently every single
> >>> frequency in the cpufreq list has ability to be marked as boost
> >>> frequency - if userspace does not maintain that, then, IMHO, fix
> >>> the userspace :D
> >>>
> >>
> >> /sys/devices/system/cpu/cpu*/cpufreq/scaling_available_frequencies
> >> gives the list of frequencies based on the state of the boost
> >> feature at anytime.
> >>
> >> Intuitively the list without boost shouldn't have any frequency
> >> above the range when it's enabled :), that's what I was referring
> >> to. So I am not talking about any issue with user-space
> >> maintenance.
> > Fair enough - but i still think it has nothing to do with dt binding
> > itself -> and i think the discussion we've had should be good for
> > the binding provided in this patch.. I hope.. if documentation
> > needs a bit of better explanation to prevent a repeat of the same
> > discussion at a later point of time, now might be a good time to
> > add it in.
> 
> The term boost and over-clocking have been described in the bindings
> document as being the same. Since the term over-clocking refers to
> running a CPU beyond normal operating frequencies, I tend to agree
> with Sudeep that it is not intuitive if a normal operating frequency
> is greater than a boost mode frequency.
> 
> Otherwise, when userspace does "echo 1 >
> /sys/devices/system/cpu/cpufreq/boost", what is it supposed to mean. I
> think the original intent of boost mode patches was to allow CPU to
> operate at frequencies greater than the normal operating frequencies.
> 
> Lukasz, how would you want this to be handled?

Please consider an example:

normal-freqs: 1400000, 1200000, 1000000, 800000, 600000, 400000, 200000
[1]
boost-freqs: 1700000, 1600000, 1500000. [2]

All those freqs shall be represented as OPPs freq and voltage tuple.

Best would be to specify all the boost-freqs as:
boost-freqs = <1700000 1600000 1500000> to be prepared for future
quirks or problems (or special cases which might show up latter).
If anybody can foresee any potential threads - like platform on which
boost freqs are 1700000 and 1500000, but not 1600000, then please share
this information.

However, I think that it would be also acceptable to specify only:
boost-freq = <1500000> and mark all freqs greater or equal to it as
CPUFREQ_BOOST_FREQ.

If there aren't any potential problems, then I think the second option
would be a good solution (we would have only one BOOST attribute stored
at CPUs DTS node).

> 
> Thanks,
> Thomas.
> 
> >
> > Regards,
> > Nishanth Menon
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel



-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
@ 2014-02-10  7:53                     ` Lukasz Majewski
  0 siblings, 0 replies; 54+ messages in thread
From: Lukasz Majewski @ 2014-02-10  7:53 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Thomas, Sudeep,

> On Sat, Feb 8, 2014 at 1:11 AM, Nishanth Menon <nm@ti.com> wrote:
> > On Fri, Feb 7, 2014 at 12:02 PM, Sudeep Holla
> > <Sudeep.Holla@arm.com> wrote:
> >> On 07/02/14 17:37, Nishanth Menon wrote:
> >>> On Fri, Feb 7, 2014 at 11:31 AM, Sudeep Holla
> >>> <Sudeep.Holla@arm.com> wrote:
> >>
> >> [...]
> >>
> >>>> Yes I think its counter-intuitive as it's visible to the
> >>>> userspace(list of frequencies and the boost parameters are
> >>>> exposed through sysfs)
> >>>
> >>> That will be a different problem -> as currently every single
> >>> frequency in the cpufreq list has ability to be marked as boost
> >>> frequency - if userspace does not maintain that, then, IMHO, fix
> >>> the userspace :D
> >>>
> >>
> >> /sys/devices/system/cpu/cpu*/cpufreq/scaling_available_frequencies
> >> gives the list of frequencies based on the state of the boost
> >> feature at anytime.
> >>
> >> Intuitively the list without boost shouldn't have any frequency
> >> above the range when it's enabled :), that's what I was referring
> >> to. So I am not talking about any issue with user-space
> >> maintenance.
> > Fair enough - but i still think it has nothing to do with dt binding
> > itself -> and i think the discussion we've had should be good for
> > the binding provided in this patch.. I hope.. if documentation
> > needs a bit of better explanation to prevent a repeat of the same
> > discussion at a later point of time, now might be a good time to
> > add it in.
> 
> The term boost and over-clocking have been described in the bindings
> document as being the same. Since the term over-clocking refers to
> running a CPU beyond normal operating frequencies, I tend to agree
> with Sudeep that it is not intuitive if a normal operating frequency
> is greater than a boost mode frequency.
> 
> Otherwise, when userspace does "echo 1 >
> /sys/devices/system/cpu/cpufreq/boost", what is it supposed to mean. I
> think the original intent of boost mode patches was to allow CPU to
> operate at frequencies greater than the normal operating frequencies.
> 
> Lukasz, how would you want this to be handled?

Please consider an example:

normal-freqs: 1400000, 1200000, 1000000, 800000, 600000, 400000, 200000
[1]
boost-freqs: 1700000, 1600000, 1500000. [2]

All those freqs shall be represented as OPPs freq and voltage tuple.

Best would be to specify all the boost-freqs as:
boost-freqs = <1700000 1600000 1500000> to be prepared for future
quirks or problems (or special cases which might show up latter).
If anybody can foresee any potential threads - like platform on which
boost freqs are 1700000 and 1500000, but not 1600000, then please share
this information.

However, I think that it would be also acceptable to specify only:
boost-freq = <1500000> and mark all freqs greater or equal to it as
CPUFREQ_BOOST_FREQ.

If there aren't any potential problems, then I think the second option
would be a good solution (we would have only one BOOST attribute stored
at CPUs DTS node).

> 
> Thanks,
> Thomas.
> 
> >
> > Regards,
> > Nishanth Menon
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel



-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v2 1/2] PM / OPP: Allow boost frequency to be looked up from device tree
  2014-02-08  5:10       ` Thomas Abraham
@ 2014-02-10 10:40         ` Sudeep Holla
  -1 siblings, 0 replies; 54+ messages in thread
From: Sudeep Holla @ 2014-02-10 10:40 UTC (permalink / raw)
  To: Thomas Abraham
  Cc: Sudeep.Holla, linux-pm, devicetree, linux-arm-kernel, rjw,
	linux-samsung-soc, kgene.kim, t.figa, l.majewski, viresh.kumar,
	thomas.ab, Nishanth Menon

On 08/02/14 05:10, Thomas Abraham wrote:
> On Fri, Feb 7, 2014 at 9:31 PM, Sudeep Holla <Sudeep.Holla@arm.com> wrote:
>> On 07/02/14 15:19, Thomas Abraham wrote:
>>> From: Thomas Abraham <thomas.ab@samsung.com>
>>>
>>> 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 <nm@ti.com>
>>> Cc: Lukasz Majewski <l.majewski@samsung.com>
>>> Signed-off-by: Thomas Abraham <thomas.ab@samsung.com>
>>> ---
>>>  drivers/base/power/opp.c |   34 +++++++++++++++++++++++++++++++++-
>>>  1 file changed, 33 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
>>> index fa41874..b636826 100644
>>> --- a/drivers/base/power/opp.c
>>> +++ b/drivers/base/power/opp.c
>>> @@ -628,7 +628,8 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
>>>       struct device_opp *dev_opp;
>>>       struct dev_pm_opp *opp;
>>>       struct cpufreq_frequency_table *freq_table;
>>> -     int i = 0;
>>> +     int i = 0, j, len, ret;
>>> +     u32 *boost_freqs = NULL;
>>>
>>>       /* Pretend as if I am an updater */
>>>       mutex_lock(&dev_opp_list_lock);
>>> @@ -650,10 +651,35 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
>>>               return -ENOMEM;
>>>       }
>>>
>>> +     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 err_boost;
>>> +             }
>>> +
>>> +             boost_freqs = kzalloc(len, GFP_KERNEL);
>>> +             if (!boost_freqs) {
>>> +                     dev_warn(dev, "%s: no memory for boost freq table\n",
>>> +                                     __func__);
>>> +                     ret = -ENOMEM;
>>> +                     goto err_boost;
>>> +             }
>>> +             of_property_read_u32_array(dev->of_node, "boost-frequency",
>>> +                     boost_freqs, len / sizeof(u32));
>>> +     }
>>> +
>>>       list_for_each_entry(opp, &dev_opp->opp_list, node) {
>>>               if (opp->available) {
>>>                       freq_table[i].driver_data = i;
>>>                       freq_table[i].frequency = opp->rate / 1000;
>>> +                     for (j = 0; j < len / sizeof(u32) && boost_freqs; j++) {
>>> +                             if (boost_freqs[j] == freq_table[i].frequency) {
>>> +                                     freq_table[i].driver_data =
>>> +                                                     CPUFREQ_BOOST_FREQ;
>>> +                                     break;
>>> +                             }
>>> +                     }
>>>                       i++;
>>>               }
>>>       }
>> IIRC you had mentioned that the boost-opp was not limited to be a cpufreq, but
>> this change seems to be cpufreq only.
> 
> Yes, but as you have initiated the discussion on extending the OPP
> binding, this has been limited to cpufreq only. If the new OPP library
> has support for listing boost frequency, this can be migrated to the
> new OPP libaray.
> 

Fair enough, just wanted to check as I couldn't get the info following the
thread. I might have missed it.

Regards,
Sudeep

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v2 1/2] PM / OPP: Allow boost frequency to be looked up from device tree
@ 2014-02-10 10:40         ` Sudeep Holla
  0 siblings, 0 replies; 54+ messages in thread
From: Sudeep Holla @ 2014-02-10 10:40 UTC (permalink / raw)
  To: linux-arm-kernel

On 08/02/14 05:10, Thomas Abraham wrote:
> On Fri, Feb 7, 2014 at 9:31 PM, Sudeep Holla <Sudeep.Holla@arm.com> wrote:
>> On 07/02/14 15:19, Thomas Abraham wrote:
>>> From: Thomas Abraham <thomas.ab@samsung.com>
>>>
>>> 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 <nm@ti.com>
>>> Cc: Lukasz Majewski <l.majewski@samsung.com>
>>> Signed-off-by: Thomas Abraham <thomas.ab@samsung.com>
>>> ---
>>>  drivers/base/power/opp.c |   34 +++++++++++++++++++++++++++++++++-
>>>  1 file changed, 33 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
>>> index fa41874..b636826 100644
>>> --- a/drivers/base/power/opp.c
>>> +++ b/drivers/base/power/opp.c
>>> @@ -628,7 +628,8 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
>>>       struct device_opp *dev_opp;
>>>       struct dev_pm_opp *opp;
>>>       struct cpufreq_frequency_table *freq_table;
>>> -     int i = 0;
>>> +     int i = 0, j, len, ret;
>>> +     u32 *boost_freqs = NULL;
>>>
>>>       /* Pretend as if I am an updater */
>>>       mutex_lock(&dev_opp_list_lock);
>>> @@ -650,10 +651,35 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev,
>>>               return -ENOMEM;
>>>       }
>>>
>>> +     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 err_boost;
>>> +             }
>>> +
>>> +             boost_freqs = kzalloc(len, GFP_KERNEL);
>>> +             if (!boost_freqs) {
>>> +                     dev_warn(dev, "%s: no memory for boost freq table\n",
>>> +                                     __func__);
>>> +                     ret = -ENOMEM;
>>> +                     goto err_boost;
>>> +             }
>>> +             of_property_read_u32_array(dev->of_node, "boost-frequency",
>>> +                     boost_freqs, len / sizeof(u32));
>>> +     }
>>> +
>>>       list_for_each_entry(opp, &dev_opp->opp_list, node) {
>>>               if (opp->available) {
>>>                       freq_table[i].driver_data = i;
>>>                       freq_table[i].frequency = opp->rate / 1000;
>>> +                     for (j = 0; j < len / sizeof(u32) && boost_freqs; j++) {
>>> +                             if (boost_freqs[j] == freq_table[i].frequency) {
>>> +                                     freq_table[i].driver_data =
>>> +                                                     CPUFREQ_BOOST_FREQ;
>>> +                                     break;
>>> +                             }
>>> +                     }
>>>                       i++;
>>>               }
>>>       }
>> IIRC you had mentioned that the boost-opp was not limited to be a cpufreq, but
>> this change seems to be cpufreq only.
> 
> Yes, but as you have initiated the discussion on extending the OPP
> binding, this has been limited to cpufreq only. If the new OPP library
> has support for listing boost frequency, this can be migrated to the
> new OPP libaray.
> 

Fair enough, just wanted to check as I couldn't get the info following the
thread. I might have missed it.

Regards,
Sudeep

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
  2014-02-08  6:47                 ` Thomas Abraham
@ 2014-02-10 10:46                   ` Sudeep Holla
  -1 siblings, 0 replies; 54+ messages in thread
From: Sudeep Holla @ 2014-02-10 10:46 UTC (permalink / raw)
  To: Thomas Abraham
  Cc: Sudeep.Holla, Nishanth Menon, linux-pm, devicetree,
	linux-arm-kernel, rjw, linux-samsung-soc, kgene.kim, t.figa,
	l.majewski, viresh.kumar, thomas.ab, Rob Herring, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala

On 08/02/14 06:47, Thomas Abraham wrote:
> On Fri, Feb 7, 2014 at 11:32 PM, Sudeep Holla <Sudeep.Holla@arm.com> wrote:
>> On 07/02/14 17:37, Nishanth Menon wrote:
>>> On Fri, Feb 7, 2014 at 11:31 AM, Sudeep Holla <Sudeep.Holla@arm.com> wrote:
>>
>> [...]
>>
>>>> Yes I think its counter-intuitive as it's visible to the userspace(list of
>>>> frequencies and the boost parameters are exposed through sysfs)
>>>
>>> That will be a different problem -> as currently every single
>>> frequency in the cpufreq list has ability to be marked as boost
>>> frequency - if userspace does not maintain that, then, IMHO, fix the
>>> userspace :D
>>>
>>
>> /sys/devices/system/cpu/cpu*/cpufreq/scaling_available_frequencies gives
>> the list of frequencies based on the state of the boost feature at anytime.
> 
> The list of frequencies in
> /sys/devices/system/cpu/cpu*/cpufreq/scaling_available_frequencies
> does not change based in the state of the boost feature (enabled or
> disabled). But the scaling_max_frequency and scaling_min_frequency are
> updated based on the set of available + boost frequencies available.
> 

Ah ok, sorry just glanced the code and misunderstood it. It make sense to update
only max_frequency.

Regards,
Sudeep

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
@ 2014-02-10 10:46                   ` Sudeep Holla
  0 siblings, 0 replies; 54+ messages in thread
From: Sudeep Holla @ 2014-02-10 10:46 UTC (permalink / raw)
  To: linux-arm-kernel

On 08/02/14 06:47, Thomas Abraham wrote:
> On Fri, Feb 7, 2014 at 11:32 PM, Sudeep Holla <Sudeep.Holla@arm.com> wrote:
>> On 07/02/14 17:37, Nishanth Menon wrote:
>>> On Fri, Feb 7, 2014 at 11:31 AM, Sudeep Holla <Sudeep.Holla@arm.com> wrote:
>>
>> [...]
>>
>>>> Yes I think its counter-intuitive as it's visible to the userspace(list of
>>>> frequencies and the boost parameters are exposed through sysfs)
>>>
>>> That will be a different problem -> as currently every single
>>> frequency in the cpufreq list has ability to be marked as boost
>>> frequency - if userspace does not maintain that, then, IMHO, fix the
>>> userspace :D
>>>
>>
>> /sys/devices/system/cpu/cpu*/cpufreq/scaling_available_frequencies gives
>> the list of frequencies based on the state of the boost feature at anytime.
> 
> The list of frequencies in
> /sys/devices/system/cpu/cpu*/cpufreq/scaling_available_frequencies
> does not change based in the state of the boost feature (enabled or
> disabled). But the scaling_max_frequency and scaling_min_frequency are
> updated based on the set of available + boost frequencies available.
> 

Ah ok, sorry just glanced the code and misunderstood it. It make sense to update
only max_frequency.

Regards,
Sudeep

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
  2014-02-10  7:38                   ` Lukasz Majewski
@ 2014-02-10 10:51                     ` Sudeep Holla
  -1 siblings, 0 replies; 54+ messages in thread
From: Sudeep Holla @ 2014-02-10 10:51 UTC (permalink / raw)
  To: Lukasz Majewski, Thomas Abraham
  Cc: Sudeep.Holla, Nishanth Menon, linux-pm, devicetree,
	linux-arm-kernel, rjw, linux-samsung-soc, kgene.kim, t.figa,
	viresh.kumar, thomas.ab, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala

On 10/02/14 07:38, Lukasz Majewski wrote:
> Hi Thomas, Sudeep,
> 
>> On Fri, Feb 7, 2014 at 11:32 PM, Sudeep Holla <Sudeep.Holla@arm.com>
>> wrote:
>>> On 07/02/14 17:37, Nishanth Menon wrote:
>>>> On Fri, Feb 7, 2014 at 11:31 AM, Sudeep Holla
>>>> <Sudeep.Holla@arm.com> wrote:
>>>
>>> [...]
>>>
>>>>> Yes I think its counter-intuitive as it's visible to the
>>>>> userspace(list of frequencies and the boost parameters are
>>>>> exposed through sysfs)
>>>>
>>>> That will be a different problem -> as currently every single
>>>> frequency in the cpufreq list has ability to be marked as boost
>>>> frequency - if userspace does not maintain that, then, IMHO, fix
>>>> the userspace :D
>>>>
>>>
>>> /sys/devices/system/cpu/cpu*/cpufreq/scaling_available_frequencies
>>> gives the list of frequencies based on the state of the boost
>>> feature at anytime.
>>
>> The list of frequencies in
>> /sys/devices/system/cpu/cpu*/cpufreq/scaling_available_frequencies
>> does not change based in the state of the boost feature (enabled or
>> disabled). But the scaling_max_frequency and scaling_min_frequency are
>> updated based on the set of available + boost frequencies available.
> 
> With boost intended behavior is as follow:
> 
> /sys/devices/system/cpu/cpu*/cpufreq/scaling_available_frequencies [1]
> 
> shows the non boost frequencies no matter if boost is enabled or not.
> Those are the "normal" frequencies.
> 
> When boost is supported (by enabling the CONFIG_CPU_FREQ_BOOST_SW)
> extra sysfs attribute shows up:
> 
> /sys/devices/system/cpu/cpu0/cpufreq/scaling_boost_frequencies [2]
> in which are listed only the boost frequencies.
> 

Correct, sorry I misunderstood this to dynamic change in
scaling_available_frequencies based on state of boot.

Regards,
Sudeep

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
@ 2014-02-10 10:51                     ` Sudeep Holla
  0 siblings, 0 replies; 54+ messages in thread
From: Sudeep Holla @ 2014-02-10 10:51 UTC (permalink / raw)
  To: linux-arm-kernel

On 10/02/14 07:38, Lukasz Majewski wrote:
> Hi Thomas, Sudeep,
> 
>> On Fri, Feb 7, 2014 at 11:32 PM, Sudeep Holla <Sudeep.Holla@arm.com>
>> wrote:
>>> On 07/02/14 17:37, Nishanth Menon wrote:
>>>> On Fri, Feb 7, 2014 at 11:31 AM, Sudeep Holla
>>>> <Sudeep.Holla@arm.com> wrote:
>>>
>>> [...]
>>>
>>>>> Yes I think its counter-intuitive as it's visible to the
>>>>> userspace(list of frequencies and the boost parameters are
>>>>> exposed through sysfs)
>>>>
>>>> That will be a different problem -> as currently every single
>>>> frequency in the cpufreq list has ability to be marked as boost
>>>> frequency - if userspace does not maintain that, then, IMHO, fix
>>>> the userspace :D
>>>>
>>>
>>> /sys/devices/system/cpu/cpu*/cpufreq/scaling_available_frequencies
>>> gives the list of frequencies based on the state of the boost
>>> feature at anytime.
>>
>> The list of frequencies in
>> /sys/devices/system/cpu/cpu*/cpufreq/scaling_available_frequencies
>> does not change based in the state of the boost feature (enabled or
>> disabled). But the scaling_max_frequency and scaling_min_frequency are
>> updated based on the set of available + boost frequencies available.
> 
> With boost intended behavior is as follow:
> 
> /sys/devices/system/cpu/cpu*/cpufreq/scaling_available_frequencies [1]
> 
> shows the non boost frequencies no matter if boost is enabled or not.
> Those are the "normal" frequencies.
> 
> When boost is supported (by enabling the CONFIG_CPU_FREQ_BOOST_SW)
> extra sysfs attribute shows up:
> 
> /sys/devices/system/cpu/cpu0/cpufreq/scaling_boost_frequencies [2]
> in which are listed only the boost frequencies.
> 

Correct, sorry I misunderstood this to dynamic change in
scaling_available_frequencies based on state of boot.

Regards,
Sudeep

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
  2014-02-10  7:53                     ` Lukasz Majewski
@ 2014-02-10 11:20                       ` Sudeep Holla
  -1 siblings, 0 replies; 54+ messages in thread
From: Sudeep Holla @ 2014-02-10 11:20 UTC (permalink / raw)
  To: Lukasz Majewski, Thomas Abraham
  Cc: Sudeep.Holla, Nishanth Menon, Mark Rutland, devicetree,
	linux-samsung-soc, Pawel Moll, linux-pm, viresh.kumar, t.figa,
	Ian Campbell, rjw, Rob Herring, kgene.kim, thomas.ab, Kumar Gala,
	linux-arm-kernel

On 10/02/14 07:53, Lukasz Majewski wrote:
> Hi Thomas, Sudeep,
> 
>> On Sat, Feb 8, 2014 at 1:11 AM, Nishanth Menon <nm@ti.com> wrote:
>>> On Fri, Feb 7, 2014 at 12:02 PM, Sudeep Holla
>>> <Sudeep.Holla@arm.com> wrote:
>>>> On 07/02/14 17:37, Nishanth Menon wrote:
>>>>> On Fri, Feb 7, 2014 at 11:31 AM, Sudeep Holla
>>>>> <Sudeep.Holla@arm.com> wrote:
>>>>
>>>> [...]
>>>>
>>>>>> Yes I think its counter-intuitive as it's visible to the
>>>>>> userspace(list of frequencies and the boost parameters are
>>>>>> exposed through sysfs)
>>>>>
>>>>> That will be a different problem -> as currently every single
>>>>> frequency in the cpufreq list has ability to be marked as boost
>>>>> frequency - if userspace does not maintain that, then, IMHO, fix
>>>>> the userspace :D
>>>>>
>>>>
>>>> /sys/devices/system/cpu/cpu*/cpufreq/scaling_available_frequencies
>>>> gives the list of frequencies based on the state of the boost
>>>> feature at anytime.
>>>>
>>>> Intuitively the list without boost shouldn't have any frequency
>>>> above the range when it's enabled :), that's what I was referring
>>>> to. So I am not talking about any issue with user-space
>>>> maintenance.
>>> Fair enough - but i still think it has nothing to do with dt binding
>>> itself -> and i think the discussion we've had should be good for
>>> the binding provided in this patch.. I hope.. if documentation
>>> needs a bit of better explanation to prevent a repeat of the same
>>> discussion at a later point of time, now might be a good time to
>>> add it in.
>>
Nishanth, though I am not convinced that it should be list, since you have a
valid point that this should not prevent in having this feature, I am fine with
the list.

>> The term boost and over-clocking have been described in the bindings
>> document as being the same. Since the term over-clocking refers to
>> running a CPU beyond normal operating frequencies, I tend to agree
>> with Sudeep that it is not intuitive if a normal operating frequency
>> is greater than a boost mode frequency.
>>
>> Otherwise, when userspace does "echo 1 >
>> /sys/devices/system/cpu/cpufreq/boost", what is it supposed to mean. I
>> think the original intent of boost mode patches was to allow CPU to
>> operate at frequencies greater than the normal operating frequencies.
>>
>> Lukasz, how would you want this to be handled?
> 
> Please consider an example:
> 
> normal-freqs: 1400000, 1200000, 1000000, 800000, 600000, 400000, 200000
> [1]
> boost-freqs: 1700000, 1600000, 1500000. [2]
> 
> All those freqs shall be represented as OPPs freq and voltage tuple.
> 
> Best would be to specify all the boost-freqs as:
> boost-freqs = <1700000 1600000 1500000> to be prepared for future
> quirks or problems (or special cases which might show up latter).
> If anybody can foresee any potential threads - like platform on which
> boost freqs are 1700000 and 1500000, but not 1600000, then please share
> this information.
> 
If that's the case then why should it be included in the list of OPPs.
I know Nishanth had a valid point in other thread previously(like including
SoC.dtsi having OPPs in platform files), but that's different problem.

> However, I think that it would be also acceptable to specify only:
> boost-freq = <1500000> and mark all freqs greater or equal to it as
> CPUFREQ_BOOST_FREQ.
> 
> If there aren't any potential problems, then I think the second option
> would be a good solution (we would have only one BOOST attribute stored
> at CPUs DTS node).
> 

Yes I prefer this to keep it simple and as per the definition of overclocking
or turbo boost in hardware terms if possible.

Just another thought, not sure how much this is true for real platforms, sharing
it anyway. IIUC these boost frequencies have certain constraints like thermal
and can't be sustained all the processors for long time.

So does it make sense to specify normal max frequency instead of boost-frequency
which by definition would be: "The maximum frequency that all processors are can
sustain simultaneously without any thermal constraints".
Ignore this if this is not the definition of boost on real platforms.


Regards,
Sudeep

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
@ 2014-02-10 11:20                       ` Sudeep Holla
  0 siblings, 0 replies; 54+ messages in thread
From: Sudeep Holla @ 2014-02-10 11:20 UTC (permalink / raw)
  To: linux-arm-kernel

On 10/02/14 07:53, Lukasz Majewski wrote:
> Hi Thomas, Sudeep,
> 
>> On Sat, Feb 8, 2014 at 1:11 AM, Nishanth Menon <nm@ti.com> wrote:
>>> On Fri, Feb 7, 2014 at 12:02 PM, Sudeep Holla
>>> <Sudeep.Holla@arm.com> wrote:
>>>> On 07/02/14 17:37, Nishanth Menon wrote:
>>>>> On Fri, Feb 7, 2014 at 11:31 AM, Sudeep Holla
>>>>> <Sudeep.Holla@arm.com> wrote:
>>>>
>>>> [...]
>>>>
>>>>>> Yes I think its counter-intuitive as it's visible to the
>>>>>> userspace(list of frequencies and the boost parameters are
>>>>>> exposed through sysfs)
>>>>>
>>>>> That will be a different problem -> as currently every single
>>>>> frequency in the cpufreq list has ability to be marked as boost
>>>>> frequency - if userspace does not maintain that, then, IMHO, fix
>>>>> the userspace :D
>>>>>
>>>>
>>>> /sys/devices/system/cpu/cpu*/cpufreq/scaling_available_frequencies
>>>> gives the list of frequencies based on the state of the boost
>>>> feature at anytime.
>>>>
>>>> Intuitively the list without boost shouldn't have any frequency
>>>> above the range when it's enabled :), that's what I was referring
>>>> to. So I am not talking about any issue with user-space
>>>> maintenance.
>>> Fair enough - but i still think it has nothing to do with dt binding
>>> itself -> and i think the discussion we've had should be good for
>>> the binding provided in this patch.. I hope.. if documentation
>>> needs a bit of better explanation to prevent a repeat of the same
>>> discussion at a later point of time, now might be a good time to
>>> add it in.
>>
Nishanth, though I am not convinced that it should be list, since you have a
valid point that this should not prevent in having this feature, I am fine with
the list.

>> The term boost and over-clocking have been described in the bindings
>> document as being the same. Since the term over-clocking refers to
>> running a CPU beyond normal operating frequencies, I tend to agree
>> with Sudeep that it is not intuitive if a normal operating frequency
>> is greater than a boost mode frequency.
>>
>> Otherwise, when userspace does "echo 1 >
>> /sys/devices/system/cpu/cpufreq/boost", what is it supposed to mean. I
>> think the original intent of boost mode patches was to allow CPU to
>> operate at frequencies greater than the normal operating frequencies.
>>
>> Lukasz, how would you want this to be handled?
> 
> Please consider an example:
> 
> normal-freqs: 1400000, 1200000, 1000000, 800000, 600000, 400000, 200000
> [1]
> boost-freqs: 1700000, 1600000, 1500000. [2]
> 
> All those freqs shall be represented as OPPs freq and voltage tuple.
> 
> Best would be to specify all the boost-freqs as:
> boost-freqs = <1700000 1600000 1500000> to be prepared for future
> quirks or problems (or special cases which might show up latter).
> If anybody can foresee any potential threads - like platform on which
> boost freqs are 1700000 and 1500000, but not 1600000, then please share
> this information.
> 
If that's the case then why should it be included in the list of OPPs.
I know Nishanth had a valid point in other thread previously(like including
SoC.dtsi having OPPs in platform files), but that's different problem.

> However, I think that it would be also acceptable to specify only:
> boost-freq = <1500000> and mark all freqs greater or equal to it as
> CPUFREQ_BOOST_FREQ.
> 
> If there aren't any potential problems, then I think the second option
> would be a good solution (we would have only one BOOST attribute stored
> at CPUs DTS node).
> 

Yes I prefer this to keep it simple and as per the definition of overclocking
or turbo boost in hardware terms if possible.

Just another thought, not sure how much this is true for real platforms, sharing
it anyway. IIUC these boost frequencies have certain constraints like thermal
and can't be sustained all the processors for long time.

So does it make sense to specify normal max frequency instead of boost-frequency
which by definition would be: "The maximum frequency that all processors are can
sustain simultaneously without any thermal constraints".
Ignore this if this is not the definition of boost on real platforms.


Regards,
Sudeep

^ permalink raw reply	[flat|nested] 54+ messages in thread

end of thread, other threads:[~2014-02-10 11:20 UTC | newest]

Thread overview: 54+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-07 15:19 [PATCH v2 0/2] Add device tree based lookup of boost mode frequency Thomas Abraham
2014-02-07 15:19 ` Thomas Abraham
2014-02-07 15:19 ` [PATCH v2 1/2] PM / OPP: Allow boost frequency to be looked up from device tree Thomas Abraham
2014-02-07 15:19   ` Thomas Abraham
2014-02-07 15:29   ` Nishanth Menon
2014-02-07 15:29     ` Nishanth Menon
2014-02-07 15:38     ` Thomas Abraham
2014-02-07 15:38       ` Thomas Abraham
2014-02-07 15:52       ` Nishanth Menon
2014-02-07 15:52         ` Nishanth Menon
2014-02-07 16:01   ` Sudeep Holla
2014-02-07 16:01     ` Sudeep Holla
2014-02-08  5:10     ` Thomas Abraham
2014-02-08  5:10       ` Thomas Abraham
2014-02-10  7:10       ` Lukasz Majewski
2014-02-10  7:10         ` Lukasz Majewski
2014-02-10 10:40       ` Sudeep Holla
2014-02-10 10:40         ` Sudeep Holla
2014-02-07 15:19 ` [PATCH v2 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency Thomas Abraham
2014-02-07 15:19   ` Thomas Abraham
2014-02-07 15:27   ` Nishanth Menon
2014-02-07 15:27     ` Nishanth Menon
2014-02-07 15:38     ` Thomas Abraham
2014-02-07 15:38       ` Thomas Abraham
2014-02-07 16:15   ` Sudeep Holla
2014-02-07 16:15     ` Sudeep Holla
2014-02-07 16:28     ` Sudeep Holla
2014-02-07 16:28       ` Sudeep Holla
2014-02-07 16:43       ` Nishanth Menon
2014-02-07 16:43         ` Nishanth Menon
2014-02-07 17:31         ` Sudeep Holla
2014-02-07 17:31           ` Sudeep Holla
2014-02-07 17:37           ` Nishanth Menon
2014-02-07 17:37             ` Nishanth Menon
2014-02-07 17:40             ` Nishanth Menon
2014-02-07 17:40               ` Nishanth Menon
2014-02-07 18:02             ` Sudeep Holla
2014-02-07 18:02               ` Sudeep Holla
2014-02-07 19:41               ` Nishanth Menon
2014-02-07 19:41                 ` Nishanth Menon
2014-02-08  6:47                 ` Thomas Abraham
2014-02-08  6:47                   ` Thomas Abraham
2014-02-10  7:53                   ` Lukasz Majewski
2014-02-10  7:53                     ` Lukasz Majewski
2014-02-10 11:20                     ` Sudeep Holla
2014-02-10 11:20                       ` Sudeep Holla
2014-02-08  6:47               ` Thomas Abraham
2014-02-08  6:47                 ` Thomas Abraham
2014-02-10  7:38                 ` Lukasz Majewski
2014-02-10  7:38                   ` Lukasz Majewski
2014-02-10 10:51                   ` Sudeep Holla
2014-02-10 10:51                     ` Sudeep Holla
2014-02-10 10:46                 ` Sudeep Holla
2014-02-10 10:46                   ` Sudeep Holla

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.