[RFC 0/2] extends OPP for voltage ranges

[RFC 0/2] extends OPP for voltage ranges

[Lucas Stach]

For a lot of components we don't have a strict one to one
relation between the operating frequency of the component
and a single voltage, but rather a range of valid voltages.

Most commonly those are defined as a minimum voltage needed,
a typical (or nominal) voltage chosen by the component vendor
for optimum performance and an absolute maximum voltage that
may not be exceeded without damaging the component.

Currently the OPP framework defines it's single voltage to be
the minimum voltage required for an OPP, although people have
started to implicitly redefine it to be the nominal voltage as
this gives some safety margin in the general case.

Also cpufreq drivers have started to add (or substract) a
abitrary "voltage tolerance" to the supplied values as the
connected regulators are often not able to supply the exact
specified voltage.

Rather than pushing more such workarounds into drivers, we
should enable the OPP framework to handle voltage ranges,
so we don't need any abitrary tolerances in places where we
instead could use well defined values from the component
datasheet.

The follwing 2 patches implement this handling without changing
the current behavior, but I already have a new cpufrew driver
making use the of the extended functionality that I'll post
later if you agree that the general direction of those patches
make sense.

Lucas Stach (2):
  PM / OPP: allow to use voltage ranges
  PM / OPP: extend DT parsing to allow voltage ranges

 Documentation/devicetree/bindings/power/opp.txt | 23 ++++++++++
 arch/arm/mach-omap2/opp.c                       |  3 +-
 arch/arm/mach-omap2/pm.c                        |  2 +-
 arch/arm/mach-vexpress/spc.c                    |  3 +-
 drivers/base/power/opp.c                        | 61 +++++++++++++++++++------
 drivers/cpufreq/cpufreq-cpu0.c                  |  6 +--
 drivers/cpufreq/exynos5440-cpufreq.c            |  2 +-
 drivers/cpufreq/imx6q-cpufreq.c                 |  6 +--
 drivers/cpufreq/omap-cpufreq.c                  |  2 +-
 drivers/devfreq/exynos/exynos4_bus.c            | 12 +++--
 drivers/devfreq/exynos/exynos5_bus.c            |  8 ++--
 include/linux/pm_opp.h                          | 19 ++++++--
 12 files changed, 111 insertions(+), 36 deletions(-)

-- 
2.0.0.rc0

[RFC 1/2] PM / OPP: allow to use voltage ranges

[Lucas Stach]

In many cases we don't have a strict one to one relation
between a frequency and a voltage, but rather an operating
frequency plus a voltage range that need to be maintained
for this one frequency.

This patch does not change any behavior, but only expands
the API to cope with voltage ranges.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 arch/arm/mach-omap2/opp.c            |  3 ++-
 arch/arm/mach-omap2/pm.c             |  2 +-
 arch/arm/mach-vexpress/spc.c         |  3 ++-
 drivers/base/power/opp.c             | 32 ++++++++++++++++++++++++--------
 drivers/cpufreq/cpufreq-cpu0.c       |  6 +++---
 drivers/cpufreq/exynos5440-cpufreq.c |  2 +-
 drivers/cpufreq/imx6q-cpufreq.c      |  6 +++---
 drivers/cpufreq/omap-cpufreq.c       |  2 +-
 drivers/devfreq/exynos/exynos4_bus.c | 12 +++++++++---
 drivers/devfreq/exynos/exynos5_bus.c |  8 +++++---
 include/linux/pm_opp.h               | 19 +++++++++++++++----
 11 files changed, 66 insertions(+), 29 deletions(-)

diff --git a/arch/arm/mach-omap2/opp.c b/arch/arm/mach-omap2/opp.c
index a358a07e18f2..bdd4f7fe7329 100644
--- a/arch/arm/mach-omap2/opp.c
+++ b/arch/arm/mach-omap2/opp.c
@@ -85,7 +85,8 @@ int __init omap_init_opp_table(struct omap_opp_def *opp_def,
 			dev = &oh->od->pdev->dev;
 		}
 
-		r = dev_pm_opp_add(dev, opp_def->freq, opp_def->u_volt);
+		r = dev_pm_opp_add(dev, opp_def->freq, opp_def->u_volt,
+				   opp_def->u_volt, opp_def->u_volt);
 		if (r) {
 			dev_err(dev, "%s: add OPP %ld failed for %s [%d] result=%d\n",
 				__func__, opp_def->freq,
diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
index e1b41416fbf1..761e530d835b 100644
--- a/arch/arm/mach-omap2/pm.c
+++ b/arch/arm/mach-omap2/pm.c
@@ -180,7 +180,7 @@ static int __init omap2_set_init_voltage(char *vdd_name, char *clk_name,
 		goto exit;
 	}
 
-	bootup_volt = dev_pm_opp_get_voltage(opp);
+	bootup_volt = dev_pm_opp_get_voltage(opp, OPP_VOLTAGE_NOMINAL);
 	rcu_read_unlock();
 	if (!bootup_volt) {
 		pr_err("%s: unable to find voltage corresponding to the bootup OPP for vdd_%s\n",
diff --git a/arch/arm/mach-vexpress/spc.c b/arch/arm/mach-vexpress/spc.c
index c26ef5b92ca7..bd32b3ee1111 100644
--- a/arch/arm/mach-vexpress/spc.c
+++ b/arch/arm/mach-vexpress/spc.c
@@ -431,7 +431,8 @@ static int ve_init_opp_table(struct device *cpu_dev)
 	struct ve_spc_opp *opps = info->opps[cluster];
 
 	for (idx = 0; idx < max_opp; idx++, opps++) {
-		ret = dev_pm_opp_add(cpu_dev, opps->freq * 1000, opps->u_volt);
+		ret = dev_pm_opp_add(cpu_dev, opps->freq * 1000, opps->u_volt,
+				     opps->u_volt, opps->u_volt);
 		if (ret) {
 			dev_warn(cpu_dev, "failed to add opp %lu %lu\n",
 				 opps->freq, opps->u_volt);
diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
index 25538675d59e..e738a37df915 100644
--- a/drivers/base/power/opp.c
+++ b/drivers/base/power/opp.c
@@ -63,7 +63,7 @@ struct dev_pm_opp {
 
 	bool available;
 	unsigned long rate;
-	unsigned long u_volt;
+	unsigned long u_volt_min, u_volt_nominal, u_volt_max;
 
 	struct device_opp *dev_opp;
 	struct rcu_head head;
@@ -149,16 +149,28 @@ static struct device_opp *find_device_opp(struct device *dev)
  * prior to unlocking with rcu_read_unlock() to maintain the integrity of the
  * pointer.
  */
-unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp)
+unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp,
+				     enum dev_pm_opp_voltage_type type)
 {
 	struct dev_pm_opp *tmp_opp;
 	unsigned long v = 0;
 
 	tmp_opp = rcu_dereference(opp);
-	if (unlikely(IS_ERR_OR_NULL(tmp_opp)) || !tmp_opp->available)
+	if (unlikely(IS_ERR_OR_NULL(tmp_opp)) || !tmp_opp->available) {
 		pr_err("%s: Invalid parameters\n", __func__);
-	else
-		v = tmp_opp->u_volt;
+	} else {
+		switch (type) {
+		case OPP_VOLTAGE_MIN:
+			v = tmp_opp->u_volt_min;
+			break;
+		case OPP_VOLTAGE_NOMINAL:
+			v = tmp_opp->u_volt_nominal;
+			break;
+		case OPP_VOLTAGE_MAX:
+			v = tmp_opp->u_volt_max;
+			break;
+		}
+	}
 
 	return v;
 }
@@ -395,7 +407,9 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_find_freq_floor);
  * that this function is *NOT* called under RCU protection or in contexts where
  * mutex cannot be locked.
  */
-int dev_pm_opp_add(struct device *dev, unsigned long freq, unsigned long u_volt)
+int dev_pm_opp_add(struct device *dev, unsigned long freq,
+		   unsigned long u_volt_min, unsigned long u_volt_nominal,
+		   unsigned long u_volt_max)
 {
 	struct device_opp *dev_opp = NULL;
 	struct dev_pm_opp *opp, *new_opp;
@@ -440,7 +454,9 @@ int dev_pm_opp_add(struct device *dev, unsigned long freq, unsigned long u_volt)
 	/* populate the opp table */
 	new_opp->dev_opp = dev_opp;
 	new_opp->rate = freq;
-	new_opp->u_volt = u_volt;
+	new_opp->u_volt_min = u_volt_min;
+	new_opp->u_volt_nominal = u_volt_nominal;
+	new_opp->u_volt_max = u_volt_max;
 	new_opp->available = true;
 
 	/* Insert new OPP in order of increasing frequency */
@@ -734,7 +750,7 @@ int of_init_opp_table(struct device *dev)
 		unsigned long freq = be32_to_cpup(val++) * 1000;
 		unsigned long volt = be32_to_cpup(val++);
 
-		if (dev_pm_opp_add(dev, freq, volt)) {
+		if (dev_pm_opp_add(dev, freq, volt, volt, volt)) {
 			dev_warn(dev, "%s: Failed to add OPP %ld\n",
 				 __func__, freq);
 			continue;
diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
index 1bf6bbac3e03..6678860ac1a9 100644
--- a/drivers/cpufreq/cpufreq-cpu0.c
+++ b/drivers/cpufreq/cpufreq-cpu0.c
@@ -58,7 +58,7 @@ static int cpu0_set_target(struct cpufreq_policy *policy, unsigned int index)
 			pr_err("failed to find OPP for %ld\n", freq_Hz);
 			return PTR_ERR(opp);
 		}
-		volt = dev_pm_opp_get_voltage(opp);
+		volt = dev_pm_opp_get_voltage(opp, OPP_VOLTAGE_NOMINAL);
 		rcu_read_unlock();
 		tol = volt * voltage_tolerance / 100;
 		volt_old = regulator_get_voltage(cpu_reg);
@@ -184,10 +184,10 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev)
 		rcu_read_lock();
 		opp = dev_pm_opp_find_freq_exact(cpu_dev,
 				freq_table[0].frequency * 1000, true);
-		min_uV = dev_pm_opp_get_voltage(opp);
+		min_uV = dev_pm_opp_get_voltage(opp, OPP_VOLTAGE_NOMINAL);
 		opp = dev_pm_opp_find_freq_exact(cpu_dev,
 				freq_table[i-1].frequency * 1000, true);
-		max_uV = dev_pm_opp_get_voltage(opp);
+		max_uV = dev_pm_opp_get_voltage(opp, OPP_VOLTAGE_NOMINAL);
 		rcu_read_unlock();
 		ret = regulator_set_voltage_time(cpu_reg, min_uV, max_uV);
 		if (ret > 0)
diff --git a/drivers/cpufreq/exynos5440-cpufreq.c b/drivers/cpufreq/exynos5440-cpufreq.c
index a6b8214d7b77..c223c9bbbb35 100644
--- a/drivers/cpufreq/exynos5440-cpufreq.c
+++ b/drivers/cpufreq/exynos5440-cpufreq.c
@@ -141,7 +141,7 @@ static int init_div_table(void)
 					<< P0_7_CSCLKDEV_SHIFT;
 
 		/* Calculate EMA */
-		volt_id = dev_pm_opp_get_voltage(opp);
+		volt_id = dev_pm_opp_get_voltage(opp, OPP_VOLTAGE_NOMINAL);
 		volt_id = (MAX_VOLTAGE - volt_id) / VOLTAGE_STEP;
 		if (volt_id < PMIC_HIGH_VOLT) {
 			ema_div = (CPUEMA_HIGH << P0_7_CPUEMA_SHIFT) |
diff --git a/drivers/cpufreq/imx6q-cpufreq.c b/drivers/cpufreq/imx6q-cpufreq.c
index e27fca86fe4f..91bf75df25fd 100644
--- a/drivers/cpufreq/imx6q-cpufreq.c
+++ b/drivers/cpufreq/imx6q-cpufreq.c
@@ -57,7 +57,7 @@ static int imx6q_set_target(struct cpufreq_policy *policy, unsigned int index)
 		return PTR_ERR(opp);
 	}
 
-	volt = dev_pm_opp_get_voltage(opp);
+	volt = dev_pm_opp_get_voltage(opp, OPP_VOLTAGE_NOMINAL);
 	rcu_read_unlock();
 	volt_old = regulator_get_voltage(arm_reg);
 
@@ -281,10 +281,10 @@ soc_opp_out:
 	rcu_read_lock();
 	opp = dev_pm_opp_find_freq_exact(cpu_dev,
 				  freq_table[0].frequency * 1000, true);
-	min_volt = dev_pm_opp_get_voltage(opp);
+	min_volt = dev_pm_opp_get_voltage(opp, OPP_VOLTAGE_NOMINAL);
 	opp = dev_pm_opp_find_freq_exact(cpu_dev,
 				  freq_table[--num].frequency * 1000, true);
-	max_volt = dev_pm_opp_get_voltage(opp);
+	max_volt = dev_pm_opp_get_voltage(opp, OPP_VOLTAGE_NOMINAL);
 	rcu_read_unlock();
 	ret = regulator_set_voltage_time(arm_reg, min_volt, max_volt);
 	if (ret > 0)
diff --git a/drivers/cpufreq/omap-cpufreq.c b/drivers/cpufreq/omap-cpufreq.c
index 5f69c9aa703c..f1390e6d596f 100644
--- a/drivers/cpufreq/omap-cpufreq.c
+++ b/drivers/cpufreq/omap-cpufreq.c
@@ -68,7 +68,7 @@ static int omap_target(struct cpufreq_policy *policy, unsigned int index)
 				__func__, new_freq);
 			return -EINVAL;
 		}
-		volt = dev_pm_opp_get_voltage(opp);
+		volt = dev_pm_opp_get_voltage(opp, OPP_VOLTAGE_NOMINAL);
 		rcu_read_unlock();
 		tol = volt * OPP_TOLERANCE / 100;
 		volt_old = regulator_get_voltage(mpu_reg);
diff --git a/drivers/devfreq/exynos/exynos4_bus.c b/drivers/devfreq/exynos/exynos4_bus.c
index e07b0c68c715..17fc2031509a 100644
--- a/drivers/devfreq/exynos/exynos4_bus.c
+++ b/drivers/devfreq/exynos/exynos4_bus.c
@@ -651,7 +651,7 @@ static int exynos4_bus_target(struct device *dev, unsigned long *_freq,
 		return PTR_ERR(opp);
 	}
 	new_oppinfo.rate = dev_pm_opp_get_freq(opp);
-	new_oppinfo.volt = dev_pm_opp_get_voltage(opp);
+	new_oppinfo.volt = dev_pm_opp_get_voltage(opp, OPP_VOLTAGE_NOMINAL);
 	rcu_read_unlock();
 	freq = new_oppinfo.rate;
 
@@ -874,6 +874,8 @@ static int exynos4210_init_tables(struct busfreq_data *data)
 
 	for (i = LV_0; i < EX4210_LV_NUM; i++) {
 		err = dev_pm_opp_add(data->dev, exynos4210_busclk_table[i].clk,
+			      exynos4210_busclk_table[i].volt,
+			      exynos4210_busclk_table[i].volt,
 			      exynos4210_busclk_table[i].volt);
 		if (err) {
 			dev_err(data->dev, "Cannot add opp entries.\n");
@@ -941,6 +943,8 @@ static int exynos4x12_init_tables(struct busfreq_data *data)
 
 	for (i = 0; i < EX4x12_LV_NUM; i++) {
 		ret = dev_pm_opp_add(data->dev, exynos4x12_mifclk_table[i].clk,
+			      exynos4x12_mifclk_table[i].volt,
+			      exynos4x12_mifclk_table[i].volt,
 			      exynos4x12_mifclk_table[i].volt);
 		if (ret) {
 			dev_err(data->dev, "Fail to add opp entries.\n");
@@ -978,7 +982,8 @@ static int exynos4_busfreq_pm_notifier_event(struct notifier_block *this,
 			return PTR_ERR(opp);
 		}
 		new_oppinfo.rate = dev_pm_opp_get_freq(opp);
-		new_oppinfo.volt = dev_pm_opp_get_voltage(opp);
+		new_oppinfo.volt = dev_pm_opp_get_voltage(opp,
+					OPP_VOLTAGE_NOMINAL);
 		rcu_read_unlock();
 
 		err = exynos4_bus_setvolt(data, &new_oppinfo,
@@ -1074,7 +1079,8 @@ static int exynos4_busfreq_probe(struct platform_device *pdev)
 		return PTR_ERR(opp);
 	}
 	data->curr_oppinfo.rate = dev_pm_opp_get_freq(opp);
-	data->curr_oppinfo.volt = dev_pm_opp_get_voltage(opp);
+	data->curr_oppinfo.volt = dev_pm_opp_get_voltage(opp,
+					OPP_VOLTAGE_NOMINAL);
 	rcu_read_unlock();
 
 	platform_set_drvdata(pdev, data);
diff --git a/drivers/devfreq/exynos/exynos5_bus.c b/drivers/devfreq/exynos/exynos5_bus.c
index 6eef1f7397c6..ab8a49e0b770 100644
--- a/drivers/devfreq/exynos/exynos5_bus.c
+++ b/drivers/devfreq/exynos/exynos5_bus.c
@@ -144,7 +144,7 @@ static int exynos5_busfreq_int_target(struct device *dev, unsigned long *_freq,
 	}
 
 	freq = dev_pm_opp_get_freq(opp);
-	volt = dev_pm_opp_get_voltage(opp);
+	volt = dev_pm_opp_get_voltage(opp, OPP_VOLTAGE_NOMINAL);
 	rcu_read_unlock();
 
 	old_freq = data->curr_freq;
@@ -246,6 +246,8 @@ static int exynos5250_init_int_tables(struct busfreq_data_int *data)
 
 	for (i = LV_0; i < _LV_END; i++) {
 		err = dev_pm_opp_add(data->dev, exynos5_int_opp_table[i].clk,
+				exynos5_int_opp_table[i].volt,
+				exynos5_int_opp_table[i].volt,
 				exynos5_int_opp_table[i].volt);
 		if (err) {
 			dev_err(data->dev, "Cannot add opp entries.\n");
@@ -282,7 +284,7 @@ static int exynos5_busfreq_int_pm_notifier_event(struct notifier_block *this,
 			goto unlock;
 		}
 		freq = dev_pm_opp_get_freq(opp);
-		volt = dev_pm_opp_get_voltage(opp);
+		volt = dev_pm_opp_get_voltage(opp, OPP_VOLTAGE_NOMINAL);
 		rcu_read_unlock();
 
 		err = exynos5_int_setvolt(data, volt);
@@ -374,7 +376,7 @@ static int exynos5_busfreq_int_probe(struct platform_device *pdev)
 		return PTR_ERR(opp);
 	}
 	initial_freq = dev_pm_opp_get_freq(opp);
-	initial_volt = dev_pm_opp_get_voltage(opp);
+	initial_volt = dev_pm_opp_get_voltage(opp, OPP_VOLTAGE_NOMINAL);
 	rcu_read_unlock();
 	data->curr_freq = initial_freq;
 
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index 5151b0059585..308902606caa 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -25,9 +25,16 @@ enum dev_pm_opp_event {
 	OPP_EVENT_ADD, OPP_EVENT_ENABLE, OPP_EVENT_DISABLE,
 };
 
+enum dev_pm_opp_voltage_type {
+	OPP_VOLTAGE_MIN,
+	OPP_VOLTAGE_NOMINAL,
+	OPP_VOLTAGE_MAX,
+};
+
 #if defined(CONFIG_PM_OPP)
 
-unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp);
+unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp,
+				     enum dev_pm_opp_voltage_type type);
 
 unsigned long dev_pm_opp_get_freq(struct dev_pm_opp *opp);
 
@@ -44,7 +51,8 @@ struct dev_pm_opp *dev_pm_opp_find_freq_ceil(struct device *dev,
 					     unsigned long *freq);
 
 int dev_pm_opp_add(struct device *dev, unsigned long freq,
-		   unsigned long u_volt);
+		   unsigned long u_volt_min, unsigned long u_volt_nominal,
+		   unsigned long u_volt_max);
 
 int dev_pm_opp_enable(struct device *dev, unsigned long freq);
 
@@ -52,7 +60,8 @@ int dev_pm_opp_disable(struct device *dev, unsigned long freq);
 
 struct srcu_notifier_head *dev_pm_opp_get_notifier(struct device *dev);
 #else
-static inline unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp)
+static inline unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp,
+					enum dev_pm_opp_voltage_type type)
 {
 	return 0;
 }
@@ -86,7 +95,9 @@ static inline struct dev_pm_opp *dev_pm_opp_find_freq_ceil(struct device *dev,
 }
 
 static inline int dev_pm_opp_add(struct device *dev, unsigned long freq,
-					unsigned long u_volt)
+					unsigned long u_volt_min,
+					unsigned long u_volt_nominal,
+					unsigned long u_volt_max)
 {
 	return -EINVAL;
 }
-- 
2.0.0.rc0

[RFC 2/2] PM / OPP: extend DT parsing to allow voltage ranges

[Lucas Stach]

Following the introduction of voltage ranges into OPP
we need a way to encode them in the device tree in a
similar fashion to the non-ranged versions.

To keep compatibility with old DTs the parsing function
is changed to understand both versions.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 Documentation/devicetree/bindings/power/opp.txt | 23 ++++++++++++++++++
 drivers/base/power/opp.c                        | 31 ++++++++++++++++++-------
 2 files changed, 46 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/power/opp.txt b/Documentation/devicetree/bindings/power/opp.txt
index 74499e5033fc..5b520ff321f5 100644
--- a/Documentation/devicetree/bindings/power/opp.txt
+++ b/Documentation/devicetree/bindings/power/opp.txt
@@ -10,6 +10,16 @@ Properties:
 	freq: clock frequency in kHz
 	vol: voltage in microvolt
 
+or
+
+- operating-points-range: An array of 4-tuple items, each item consisting
+  of a frequency and a related voltage range in the following form:
+  <freq min-vol-uV nom-vol-uV max-vol-uV>
+	freq: clock frequency in kHz
+	min-vol-uV: absolute minimum required voltage for this frequency
+	nom-vol-uV: nominal voltage for this frequency
+	max-vol-uV: absolute maximum allowed voltage for this frequency
+
 Examples:
 
 cpu@0 {
@@ -23,3 +33,16 @@ cpu@0 {
 		198000  850000
 	>;
 };
+
+cpu@0 {
+	compatible = "arm,cortex-a8";
+	reg = <0x0>;
+	operating-points-range = <
+		/*  kHz min(uV) nom(uV) max(uV) */
+		 166666  850000  900000 1400000
+		 400000  900000  950000 1400000
+		 800000 1050000 1100000 1400000
+		1000000 1200000 1250000 1400000
+		1200000 1300000 1350000 1400000
+	>;
+};
diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
index e738a37df915..6af9c465c46b 100644
--- a/drivers/base/power/opp.c
+++ b/drivers/base/power/opp.c
@@ -727,11 +727,16 @@ int of_init_opp_table(struct device *dev)
 {
 	const struct property *prop;
 	const __be32 *val;
-	int nr;
+	int nr, prop_size;
 
-	prop = of_find_property(dev->of_node, "operating-points", NULL);
-	if (!prop)
+	if ((prop = of_find_property(dev->of_node, "operating-points", NULL)))
+		prop_size = 2;
+	else if ((prop = of_find_property(dev->of_node,
+					  "operating-points-range", NULL)))
+		prop_size = 4;
+	else
 		return -ENODEV;
+
 	if (!prop->value)
 		return -ENODATA;
 
@@ -740,22 +745,32 @@ int of_init_opp_table(struct device *dev)
 	 * voltage like <freq-kHz vol-uV>.
 	 */
 	nr = prop->length / sizeof(u32);
-	if (nr % 2) {
+	if (nr % prop_size) {
 		dev_err(dev, "%s: Invalid OPP list\n", __func__);
 		return -EINVAL;
 	}
 
 	val = prop->value;
 	while (nr) {
-		unsigned long freq = be32_to_cpup(val++) * 1000;
-		unsigned long volt = be32_to_cpup(val++);
+		unsigned long freq, volt_min, volt_nominal, volt_max;
+
+		freq = be32_to_cpup(val++) * 1000;
+		if (prop_size == 4) {
+			volt_min = be32_to_cpup(val++);
+			volt_nominal = be32_to_cpup(val++);
+			volt_max = be32_to_cpup(val++);
+		} else {
+			volt_nominal = be32_to_cpup(val++);
+			volt_min = volt_max = volt_nominal;
+		}
 
-		if (dev_pm_opp_add(dev, freq, volt, volt, volt)) {
+		if (dev_pm_opp_add(dev, freq, volt_min, volt_nominal,
+				   volt_max)) {
 			dev_warn(dev, "%s: Failed to add OPP %ld\n",
 				 __func__, freq);
 			continue;
 		}
-		nr -= 2;
+		nr -= prop_size;
 	}
 
 	return 0;
-- 
2.0.0.rc0

Re: [RFC 2/2] PM / OPP: extend DT parsing to allow voltage ranges

[Nishanth Menon]

On 05/20/2014 09:27 AM, Lucas Stach wrote:
> Following the introduction of voltage ranges into OPP
> we need a way to encode them in the device tree in a
> similar fashion to the non-ranged versions.
> 
> To keep compatibility with old DTs the parsing function
> is changed to understand both versions.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>  Documentation/devicetree/bindings/power/opp.txt | 23 ++++++++++++++++++
>  drivers/base/power/opp.c                        | 31 ++++++++++++++++++-------
>  2 files changed, 46 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/power/opp.txt b/Documentation/devicetree/bindings/power/opp.txt
> index 74499e5033fc..5b520ff321f5 100644
> --- a/Documentation/devicetree/bindings/power/opp.txt
> +++ b/Documentation/devicetree/bindings/power/opp.txt
> @@ -10,6 +10,16 @@ Properties:
>  	freq: clock frequency in kHz
>  	vol: voltage in microvolt
>  
> +or
> +
> +- operating-points-range: An array of 4-tuple items, each item consisting
> +  of a frequency and a related voltage range in the following form:
> +  <freq min-vol-uV nom-vol-uV max-vol-uV>
> +	freq: clock frequency in kHz
> +	min-vol-uV: absolute minimum required voltage for this frequency
> +	nom-vol-uV: nominal voltage for this frequency
> +	max-vol-uV: absolute maximum allowed voltage for this frequency

And, Why cant we function at min-volt-uV? because PMIC cannot support
it? then why add voltage tolerance? This is not clear in the dt
description.



> +
>  Examples:
>  
>  cpu@0 {
> @@ -23,3 +33,16 @@ cpu@0 {
>  		198000  850000
>  	>;
>  };
> +
> +cpu@0 {
> +	compatible = "arm,cortex-a8";
> +	reg = <0x0>;
> +	operating-points-range = <
> +		/*  kHz min(uV) nom(uV) max(uV) */
> +		 166666  850000  900000 1400000
> +		 400000  900000  950000 1400000
> +		 800000 1050000 1100000 1400000
> +		1000000 1200000 1250000 1400000
> +		1200000 1300000 1350000 1400000
> +	>;
> +};

[...]

-- 
Regards,
Nishanth Menon

Re: [RFC 2/2] PM / OPP: extend DT parsing to allow voltage ranges

[Lucas Stach]

Am Dienstag, den 20.05.2014, 09:32 -0500 schrieb Nishanth Menon:
> On 05/20/2014 09:27 AM, Lucas Stach wrote:
> > Following the introduction of voltage ranges into OPP
> > we need a way to encode them in the device tree in a
> > similar fashion to the non-ranged versions.
> > 
> > To keep compatibility with old DTs the parsing function
> > is changed to understand both versions.
> > 
> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > ---
> >  Documentation/devicetree/bindings/power/opp.txt | 23 ++++++++++++++++++
> >  drivers/base/power/opp.c                        | 31 ++++++++++++++++++-------
> >  2 files changed, 46 insertions(+), 8 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/power/opp.txt b/Documentation/devicetree/bindings/power/opp.txt
> > index 74499e5033fc..5b520ff321f5 100644
> > --- a/Documentation/devicetree/bindings/power/opp.txt
> > +++ b/Documentation/devicetree/bindings/power/opp.txt
> > @@ -10,6 +10,16 @@ Properties:
> >  	freq: clock frequency in kHz
> >  	vol: voltage in microvolt
> >  
> > +or
> > +
> > +- operating-points-range: An array of 4-tuple items, each item consisting
> > +  of a frequency and a related voltage range in the following form:
> > +  <freq min-vol-uV nom-vol-uV max-vol-uV>
> > +	freq: clock frequency in kHz
> > +	min-vol-uV: absolute minimum required voltage for this frequency
> > +	nom-vol-uV: nominal voltage for this frequency
> > +	max-vol-uV: absolute maximum allowed voltage for this frequency
> 
> And, Why cant we function at min-volt-uV? because PMIC cannot support
> it? then why add voltage tolerance? This is not clear in the dt
> description.
> 
The min-vol-uV is meant as the absolute minimum voltage where the device
is still able to work.
But still vendors are giving nominal voltages that should be met if
possible. So for example for a CPU device we might always want to try to
match the nominal voltage, but in case the regulator isn't able to
supply a this voltage it's still ok to use a voltage in the
min...nominal range.

Another case may be where we are scaling CPU voltage and have to
maintain a certain voltage difference between CPU and SoC power domain.
So there may be cases where we might decide that it's better (faster) to
scale CPU only to it's min voltage to avoid touching the SoC regulator.
> 
> 
> > +
> >  Examples:
> >  
> >  cpu@0 {
> > @@ -23,3 +33,16 @@ cpu@0 {
> >  		198000  850000
> >  	>;
> >  };
> > +
> > +cpu@0 {
> > +	compatible = "arm,cortex-a8";
> > +	reg = <0x0>;
> > +	operating-points-range = <
> > +		/*  kHz min(uV) nom(uV) max(uV) */
> > +		 166666  850000  900000 1400000
> > +		 400000  900000  950000 1400000
> > +		 800000 1050000 1100000 1400000
> > +		1000000 1200000 1250000 1400000
> > +		1200000 1300000 1350000 1400000
> > +	>;
> > +};
> 
> [...]
> 

-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

Re: [RFC 2/2] PM / OPP: extend DT parsing to allow voltage ranges

[Nishanth Menon]

On Tue, May 20, 2014 at 9:41 AM, Lucas Stach <l.stach@pengutronix.de> wrote:
> Am Dienstag, den 20.05.2014, 09:32 -0500 schrieb Nishanth Menon:
>> On 05/20/2014 09:27 AM, Lucas Stach wrote:
>> > Following the introduction of voltage ranges into OPP
>> > we need a way to encode them in the device tree in a
>> > similar fashion to the non-ranged versions.
>> >
>> > To keep compatibility with old DTs the parsing function
>> > is changed to understand both versions.
>> >
>> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
>> > ---
>> >  Documentation/devicetree/bindings/power/opp.txt | 23 ++++++++++++++++++
>> >  drivers/base/power/opp.c                        | 31 ++++++++++++++++++-------
>> >  2 files changed, 46 insertions(+), 8 deletions(-)
>> >
>> > diff --git a/Documentation/devicetree/bindings/power/opp.txt b/Documentation/devicetree/bindings/power/opp.txt
>> > index 74499e5033fc..5b520ff321f5 100644
>> > --- a/Documentation/devicetree/bindings/power/opp.txt
>> > +++ b/Documentation/devicetree/bindings/power/opp.txt
>> > @@ -10,6 +10,16 @@ Properties:
>> >     freq: clock frequency in kHz
>> >     vol: voltage in microvolt
>> >
>> > +or
>> > +
>> > +- operating-points-range: An array of 4-tuple items, each item consisting
>> > +  of a frequency and a related voltage range in the following form:
>> > +  <freq min-vol-uV nom-vol-uV max-vol-uV>
>> > +   freq: clock frequency in kHz
>> > +   min-vol-uV: absolute minimum required voltage for this frequency
>> > +   nom-vol-uV: nominal voltage for this frequency
>> > +   max-vol-uV: absolute maximum allowed voltage for this frequency
>>
>> And, Why cant we function at min-volt-uV? because PMIC cannot support
>> it? then why add voltage tolerance? This is not clear in the dt
>> description.
>>
> The min-vol-uV is meant as the absolute minimum voltage where the device
> is still able to work.
> But still vendors are giving nominal voltages that should be met if
> possible. So for example for a CPU device we might always want to try to
> match the nominal voltage, but in case the regulator isn't able to
> supply a this voltage it's still ok to use a voltage in the
> min...nominal range.

So,the chip either is supposed to function OR not function for a
frequency at a voltage right?

Then, we have a problem here -> if min_uV is selected by a regulator
(for the sake of argument), then device is expected to function. So,
then why choose nom_uV on a regulator that can do min_uV? if the
device is NOT *guaranteed* to work at min_uV, then defining min_uV as
"functional voltage if nom_uV is not available" is wrong as well - no?

Is'nt that a wrong definition?


>
> Another case may be where we are scaling CPU voltage and have to
> maintain a certain voltage difference between CPU and SoC power domain.
> So there may be cases where we might decide that it's better (faster) to
> scale CPU only to it's min voltage to avoid touching the SoC regulator.

That is not the job of OPP definition for device X - voltage domain to
voltage domain dependency is part of something else (for example: an
voltage domain driver like an RFC[1] or equivalent)

[1]
http://marc.info/?l=linux-omap&m=139275579731801&w=2

--
Regards,
Nishanth Menon

Re: [RFC 2/2] PM / OPP: extend DT parsing to allow voltage ranges

[Lucas Stach]

Am Dienstag, den 20.05.2014, 09:53 -0500 schrieb Nishanth Menon:
> On Tue, May 20, 2014 at 9:41 AM, Lucas Stach <l.stach@pengutronix.de> wrote:
> > Am Dienstag, den 20.05.2014, 09:32 -0500 schrieb Nishanth Menon:
> >> On 05/20/2014 09:27 AM, Lucas Stach wrote:
> >> > Following the introduction of voltage ranges into OPP
> >> > we need a way to encode them in the device tree in a
> >> > similar fashion to the non-ranged versions.
> >> >
> >> > To keep compatibility with old DTs the parsing function
> >> > is changed to understand both versions.
> >> >
> >> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> >> > ---
> >> >  Documentation/devicetree/bindings/power/opp.txt | 23 ++++++++++++++++++
> >> >  drivers/base/power/opp.c                        | 31 ++++++++++++++++++-------
> >> >  2 files changed, 46 insertions(+), 8 deletions(-)
> >> >
> >> > diff --git a/Documentation/devicetree/bindings/power/opp.txt b/Documentation/devicetree/bindings/power/opp.txt
> >> > index 74499e5033fc..5b520ff321f5 100644
> >> > --- a/Documentation/devicetree/bindings/power/opp.txt
> >> > +++ b/Documentation/devicetree/bindings/power/opp.txt
> >> > @@ -10,6 +10,16 @@ Properties:
> >> >     freq: clock frequency in kHz
> >> >     vol: voltage in microvolt
> >> >
> >> > +or
> >> > +
> >> > +- operating-points-range: An array of 4-tuple items, each item consisting
> >> > +  of a frequency and a related voltage range in the following form:
> >> > +  <freq min-vol-uV nom-vol-uV max-vol-uV>
> >> > +   freq: clock frequency in kHz
> >> > +   min-vol-uV: absolute minimum required voltage for this frequency
> >> > +   nom-vol-uV: nominal voltage for this frequency
> >> > +   max-vol-uV: absolute maximum allowed voltage for this frequency
> >>
> >> And, Why cant we function at min-volt-uV? because PMIC cannot support
> >> it? then why add voltage tolerance? This is not clear in the dt
> >> description.
> >>
> > The min-vol-uV is meant as the absolute minimum voltage where the device
> > is still able to work.
> > But still vendors are giving nominal voltages that should be met if
> > possible. So for example for a CPU device we might always want to try to
> > match the nominal voltage, but in case the regulator isn't able to
> > supply a this voltage it's still ok to use a voltage in the
> > min...nominal range.
> 
> So,the chip either is supposed to function OR not function for a
> frequency at a voltage right?
> 
> Then, we have a problem here -> if min_uV is selected by a regulator
> (for the sake of argument), then device is expected to function. So,
> then why choose nom_uV on a regulator that can do min_uV? if the
> device is NOT *guaranteed* to work at min_uV, then defining min_uV as
> "functional voltage if nom_uV is not available" is wrong as well - no?
> 
> Is'nt that a wrong definition?
> 
The chip (or part of it) is supposed to work at min_uV. This is the
value you can find in datasheets as the absolute minimum voltage. The
values provided as min_uV are always safe to be used.

Technically we could get away with just defining a minimum and a maximum
voltage for one OPP, but I think it's wrong to introduce a new DT
binding that already knowingly discards information which is already
present, as vendors usually define the 3-tuple of [min|typical|max] in
their component datasheets. I want to avoid introducing yet another
binding once we see a clear use-case for the nominal voltage.
 
> 
> >
> > Another case may be where we are scaling CPU voltage and have to
> > maintain a certain voltage difference between CPU and SoC power domain.
> > So there may be cases where we might decide that it's better (faster) to
> > scale CPU only to it's min voltage to avoid touching the SoC regulator.
> 
> That is not the job of OPP definition for device X - voltage domain to
> voltage domain dependency is part of something else (for example: an
> voltage domain driver like an RFC[1] or equivalent)
> 
Right, but OPP should provide the voltage domain drivers with enough
information about the devices to make some well-informed choices.
Currently the OPP framework lacks some crucial information for this.

For example you can't currently answer the question: "How much can I
raise the voltage of this domain, without exceeding the specs for any
connected device?". This feels like a major limitation.

Regards,
Lucas
-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

Re: [RFC 2/2] PM / OPP: extend DT parsing to allow voltage ranges

[Nishanth Menon]

On Tue, May 20, 2014 at 10:07 AM, Lucas Stach <l.stach@pengutronix.de> wrote:
> Am Dienstag, den 20.05.2014, 09:53 -0500 schrieb Nishanth Menon:
>> On Tue, May 20, 2014 at 9:41 AM, Lucas Stach <l.stach@pengutronix.de> wrote:
>> > Am Dienstag, den 20.05.2014, 09:32 -0500 schrieb Nishanth Menon:
>> >> On 05/20/2014 09:27 AM, Lucas Stach wrote:
>> >> > Following the introduction of voltage ranges into OPP
>> >> > we need a way to encode them in the device tree in a
>> >> > similar fashion to the non-ranged versions.
>> >> >
>> >> > To keep compatibility with old DTs the parsing function
>> >> > is changed to understand both versions.
>> >> >
>> >> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
>> >> > ---
>> >> >  Documentation/devicetree/bindings/power/opp.txt | 23 ++++++++++++++++++
>> >> >  drivers/base/power/opp.c                        | 31 ++++++++++++++++++-------
>> >> >  2 files changed, 46 insertions(+), 8 deletions(-)
>> >> >
>> >> > diff --git a/Documentation/devicetree/bindings/power/opp.txt b/Documentation/devicetree/bindings/power/opp.txt
>> >> > index 74499e5033fc..5b520ff321f5 100644
>> >> > --- a/Documentation/devicetree/bindings/power/opp.txt
>> >> > +++ b/Documentation/devicetree/bindings/power/opp.txt
>> >> > @@ -10,6 +10,16 @@ Properties:
>> >> >     freq: clock frequency in kHz
>> >> >     vol: voltage in microvolt
>> >> >
>> >> > +or
>> >> > +
>> >> > +- operating-points-range: An array of 4-tuple items, each item consisting
>> >> > +  of a frequency and a related voltage range in the following form:
>> >> > +  <freq min-vol-uV nom-vol-uV max-vol-uV>
>> >> > +   freq: clock frequency in kHz
>> >> > +   min-vol-uV: absolute minimum required voltage for this frequency
>> >> > +   nom-vol-uV: nominal voltage for this frequency
>> >> > +   max-vol-uV: absolute maximum allowed voltage for this frequency
>> >>
>> >> And, Why cant we function at min-volt-uV? because PMIC cannot support
>> >> it? then why add voltage tolerance? This is not clear in the dt
>> >> description.
>> >>
>> > The min-vol-uV is meant as the absolute minimum voltage where the device
>> > is still able to work.
>> > But still vendors are giving nominal voltages that should be met if
>> > possible. So for example for a CPU device we might always want to try to
>> > match the nominal voltage, but in case the regulator isn't able to
>> > supply a this voltage it's still ok to use a voltage in the
>> > min...nominal range.
>>
>> So,the chip either is supposed to function OR not function for a
>> frequency at a voltage right?
>>
>> Then, we have a problem here -> if min_uV is selected by a regulator
>> (for the sake of argument), then device is expected to function. So,
>> then why choose nom_uV on a regulator that can do min_uV? if the
>> device is NOT *guaranteed* to work at min_uV, then defining min_uV as
>> "functional voltage if nom_uV is not available" is wrong as well - no?
>>
>> Is'nt that a wrong definition?
>>
> The chip (or part of it) is supposed to work at min_uV. This is the
> value you can find in datasheets as the absolute minimum voltage. The
> values provided as min_uV are always safe to be used.
>

Then we just need min_uV for the OPP. nom_uV is immaterial then - let
regulator constraints define what the capabilities of the regulator
is.

> Technically we could get away with just defining a minimum and a maximum
> voltage for one OPP, but I think it's wrong to introduce a new DT
> binding that already knowingly discards information which is already
> present, as vendors usually define the 3-tuple of [min|typical|max] in
> their component datasheets. I want to avoid introducing yet another
> binding once we see a clear use-case for the nominal voltage.

Now about: max_uV seems to be maximum voltage for all OPPs
That is a absolute max that does not vary per OPP, -> is'nt that correct?

Is'nt regulator constraints supposed to entitle that?

With proper regulator constraints you dont need a max_uV, and based on
discussion above, we dont need min_uV, essentially making this binding
NOP.

>> > Another case may be where we are scaling CPU voltage and have to
>> > maintain a certain voltage difference between CPU and SoC power domain.
>> > So there may be cases where we might decide that it's better (faster) to
>> > scale CPU only to it's min voltage to avoid touching the SoC regulator.
>>
>> That is not the job of OPP definition for device X - voltage domain to
>> voltage domain dependency is part of something else (for example: an
>> voltage domain driver like an RFC[1] or equivalent)
>>
> Right, but OPP should provide the voltage domain drivers with enough
> information about the devices to make some well-informed choices.
> Currently the OPP framework lacks some crucial information for this.
>
> For example you can't currently answer the question: "How much can I
> raise the voltage of this domain, without exceeding the specs for any
> connected device?". This feels like a major limitation.
Yes addressed above.

Re: [RFC 2/2] PM / OPP: extend DT parsing to allow voltage ranges

[Nishanth Menon]

On Tue, May 20, 2014 at 10:23 AM, Nishanth Menon <nm@ti.com> wrote:
> On Tue, May 20, 2014 at 10:07 AM, Lucas Stach <l.stach@pengutronix.de> wrote:
>> Am Dienstag, den 20.05.2014, 09:53 -0500 schrieb Nishanth Menon:
>>> On Tue, May 20, 2014 at 9:41 AM, Lucas Stach <l.stach@pengutronix.de> wrote:
>>> > Am Dienstag, den 20.05.2014, 09:32 -0500 schrieb Nishanth Menon:
>>> >> On 05/20/2014 09:27 AM, Lucas Stach wrote:
>>> >> > Following the introduction of voltage ranges into OPP
>>> >> > we need a way to encode them in the device tree in a
>>> >> > similar fashion to the non-ranged versions.
>>> >> >
>>> >> > To keep compatibility with old DTs the parsing function
>>> >> > is changed to understand both versions.
>>> >> >
>>> >> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
>>> >> > ---
>>> >> >  Documentation/devicetree/bindings/power/opp.txt | 23 ++++++++++++++++++
>>> >> >  drivers/base/power/opp.c                        | 31 ++++++++++++++++++-------
>>> >> >  2 files changed, 46 insertions(+), 8 deletions(-)
>>> >> >
>>> >> > diff --git a/Documentation/devicetree/bindings/power/opp.txt b/Documentation/devicetree/bindings/power/opp.txt
>>> >> > index 74499e5033fc..5b520ff321f5 100644
>>> >> > --- a/Documentation/devicetree/bindings/power/opp.txt
>>> >> > +++ b/Documentation/devicetree/bindings/power/opp.txt
>>> >> > @@ -10,6 +10,16 @@ Properties:
>>> >> >     freq: clock frequency in kHz
>>> >> >     vol: voltage in microvolt
>>> >> >
>>> >> > +or
>>> >> > +
>>> >> > +- operating-points-range: An array of 4-tuple items, each item consisting
>>> >> > +  of a frequency and a related voltage range in the following form:
>>> >> > +  <freq min-vol-uV nom-vol-uV max-vol-uV>
>>> >> > +   freq: clock frequency in kHz
>>> >> > +   min-vol-uV: absolute minimum required voltage for this frequency
>>> >> > +   nom-vol-uV: nominal voltage for this frequency
>>> >> > +   max-vol-uV: absolute maximum allowed voltage for this frequency
>>> >>
>>> >> And, Why cant we function at min-volt-uV? because PMIC cannot support
>>> >> it? then why add voltage tolerance? This is not clear in the dt
>>> >> description.
>>> >>
>>> > The min-vol-uV is meant as the absolute minimum voltage where the device
>>> > is still able to work.
>>> > But still vendors are giving nominal voltages that should be met if
>>> > possible. So for example for a CPU device we might always want to try to
>>> > match the nominal voltage, but in case the regulator isn't able to
>>> > supply a this voltage it's still ok to use a voltage in the
>>> > min...nominal range.
>>>
>>> So,the chip either is supposed to function OR not function for a
>>> frequency at a voltage right?
>>>
>>> Then, we have a problem here -> if min_uV is selected by a regulator
>>> (for the sake of argument), then device is expected to function. So,
>>> then why choose nom_uV on a regulator that can do min_uV? if the
>>> device is NOT *guaranteed* to work at min_uV, then defining min_uV as
>>> "functional voltage if nom_uV is not available" is wrong as well - no?
>>>
>>> Is'nt that a wrong definition?
>>>
>> The chip (or part of it) is supposed to work at min_uV. This is the
>> value you can find in datasheets as the absolute minimum voltage. The
>> values provided as min_uV are always safe to be used.
>>
>
> Then we just need min_uV for the OPP. nom_uV is immaterial then - let
> regulator constraints define what the capabilities of the regulator
> is.
>
>> Technically we could get away with just defining a minimum and a maximum
>> voltage for one OPP, but I think it's wrong to introduce a new DT
>> binding that already knowingly discards information which is already
>> present, as vendors usually define the 3-tuple of [min|typical|max] in
>> their component datasheets. I want to avoid introducing yet another
>> binding once we see a clear use-case for the nominal voltage.
>
> Now about: max_uV seems to be maximum voltage for all OPPs
> That is a absolute max that does not vary per OPP, -> is'nt that correct?
>
> Is'nt regulator constraints supposed to entitle that?
>
> With proper regulator constraints you dont need a max_uV, and based on
> discussion above, we dont need min_uV, essentially making this binding

^^^ correction -> we dont need nom_uV (not min_uV)

> NOP.

so all you'd do - to give an example:

smps123_reg: smps123 {
        /* VDD_MPU */
        regulator-name = "smps123";
        regulator-min-microvolt = < 850000>;
        regulator-max-microvolt = <1250000>;
                       ^^ Absolute maximum for the voltage rail
        regulator-always-on;
        regulator-boot-on;
};

 operating-points = <
         /* kHz    uV */
         1000000 1060000
         1176000 1160000
         >;
uV in this table will be min_uV since device "cpu" is expected to
function at that voltage.

If you need to handle PMIC accuracy, use voltage-tolerance.

[snip].
---
Regards,
Nishanth Menon

Re: [RFC 2/2] PM / OPP: extend DT parsing to allow voltage ranges

[Lucas Stach]

Am Dienstag, den 20.05.2014, 10:29 -0500 schrieb Nishanth Menon:
> On Tue, May 20, 2014 at 10:23 AM, Nishanth Menon <nm@ti.com> wrote:
> > On Tue, May 20, 2014 at 10:07 AM, Lucas Stach <l.stach@pengutronix.de> wrote:
> >> Am Dienstag, den 20.05.2014, 09:53 -0500 schrieb Nishanth Menon:
> >>> On Tue, May 20, 2014 at 9:41 AM, Lucas Stach <l.stach@pengutronix.de> wrote:
> >>> > Am Dienstag, den 20.05.2014, 09:32 -0500 schrieb Nishanth Menon:
> >>> >> On 05/20/2014 09:27 AM, Lucas Stach wrote:
> >>> >> > Following the introduction of voltage ranges into OPP
> >>> >> > we need a way to encode them in the device tree in a
> >>> >> > similar fashion to the non-ranged versions.
> >>> >> >
> >>> >> > To keep compatibility with old DTs the parsing function
> >>> >> > is changed to understand both versions.
> >>> >> >
> >>> >> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> >>> >> > ---
> >>> >> >  Documentation/devicetree/bindings/power/opp.txt | 23 ++++++++++++++++++
> >>> >> >  drivers/base/power/opp.c                        | 31 ++++++++++++++++++-------
> >>> >> >  2 files changed, 46 insertions(+), 8 deletions(-)
> >>> >> >
> >>> >> > diff --git a/Documentation/devicetree/bindings/power/opp.txt b/Documentation/devicetree/bindings/power/opp.txt
> >>> >> > index 74499e5033fc..5b520ff321f5 100644
> >>> >> > --- a/Documentation/devicetree/bindings/power/opp.txt
> >>> >> > +++ b/Documentation/devicetree/bindings/power/opp.txt
> >>> >> > @@ -10,6 +10,16 @@ Properties:
> >>> >> >     freq: clock frequency in kHz
> >>> >> >     vol: voltage in microvolt
> >>> >> >
> >>> >> > +or
> >>> >> > +
> >>> >> > +- operating-points-range: An array of 4-tuple items, each item consisting
> >>> >> > +  of a frequency and a related voltage range in the following form:
> >>> >> > +  <freq min-vol-uV nom-vol-uV max-vol-uV>
> >>> >> > +   freq: clock frequency in kHz
> >>> >> > +   min-vol-uV: absolute minimum required voltage for this frequency
> >>> >> > +   nom-vol-uV: nominal voltage for this frequency
> >>> >> > +   max-vol-uV: absolute maximum allowed voltage for this frequency
> >>> >>
> >>> >> And, Why cant we function at min-volt-uV? because PMIC cannot support
> >>> >> it? then why add voltage tolerance? This is not clear in the dt
> >>> >> description.
> >>> >>
> >>> > The min-vol-uV is meant as the absolute minimum voltage where the device
> >>> > is still able to work.
> >>> > But still vendors are giving nominal voltages that should be met if
> >>> > possible. So for example for a CPU device we might always want to try to
> >>> > match the nominal voltage, but in case the regulator isn't able to
> >>> > supply a this voltage it's still ok to use a voltage in the
> >>> > min...nominal range.
> >>>
> >>> So,the chip either is supposed to function OR not function for a
> >>> frequency at a voltage right?
> >>>
> >>> Then, we have a problem here -> if min_uV is selected by a regulator
> >>> (for the sake of argument), then device is expected to function. So,
> >>> then why choose nom_uV on a regulator that can do min_uV? if the
> >>> device is NOT *guaranteed* to work at min_uV, then defining min_uV as
> >>> "functional voltage if nom_uV is not available" is wrong as well - no?
> >>>
> >>> Is'nt that a wrong definition?
> >>>
> >> The chip (or part of it) is supposed to work at min_uV. This is the
> >> value you can find in datasheets as the absolute minimum voltage. The
> >> values provided as min_uV are always safe to be used.
> >>
> >
> > Then we just need min_uV for the OPP. nom_uV is immaterial then - let
> > regulator constraints define what the capabilities of the regulator
> > is.
> >
> >> Technically we could get away with just defining a minimum and a maximum
> >> voltage for one OPP, but I think it's wrong to introduce a new DT
> >> binding that already knowingly discards information which is already
> >> present, as vendors usually define the 3-tuple of [min|typical|max] in
> >> their component datasheets. I want to avoid introducing yet another
> >> binding once we see a clear use-case for the nominal voltage.
> >
> > Now about: max_uV seems to be maximum voltage for all OPPs
> > That is a absolute max that does not vary per OPP, -> is'nt that correct?
> >
> > Is'nt regulator constraints supposed to entitle that?
> >
> > With proper regulator constraints you dont need a max_uV, and based on
> > discussion above, we dont need min_uV, essentially making this binding
> 
> ^^^ correction -> we dont need nom_uV (not min_uV)
> 
> > NOP.
> 
> so all you'd do - to give an example:
> 
> smps123_reg: smps123 {
>         /* VDD_MPU */
>         regulator-name = "smps123";
>         regulator-min-microvolt = < 850000>;
>         regulator-max-microvolt = <1250000>;
>                        ^^ Absolute maximum for the voltage rail
>         regulator-always-on;
>         regulator-boot-on;
> };
> 
>  operating-points = <
>          /* kHz    uV */
>          1000000 1060000
>          1176000 1160000
>          >;
> uV in this table will be min_uV since device "cpu" is expected to
> function at that voltage.
> 
> If you need to handle PMIC accuracy, use voltage-tolerance.
> 
And that's exactly one of the issues of the current binding, the
tolerance isn't stable, but rather per OPP.

As an example look at two operating points of the i.MX5:

/*  kHz min(uV) nom(uV) max(uV) */
 166666  850000  900000 1400000 
1200000 1300000 1350000 1400000

So for the 1,2GHz OPP the voltage tolerance is just 3,7% as anything
bigger would exceed the absolute maximum rating, but for the 166MHz OPP
it is already 64,7%.

I agree that I could just cap the rail at 1,4V and give a max voltage
equal to that as max_volt to the regulator API, but actually the device
doesn't even know it's tolerance as it has no max defined. It makes
having a max_volt in the regulator API kind of senseless, as connected
devices don't know their maximum rating.

Regards,
Lucas
-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

Re: [RFC 2/2] PM / OPP: extend DT parsing to allow voltage ranges

[Nishanth Menon]

On Tue, May 20, 2014 at 10:48 AM, Lucas Stach <l.stach@pengutronix.de> wrote:
> Am Dienstag, den 20.05.2014, 10:29 -0500 schrieb Nishanth Menon:
>> On Tue, May 20, 2014 at 10:23 AM, Nishanth Menon <nm@ti.com> wrote:

[...]
>> If you need to handle PMIC accuracy, use voltage-tolerance.
>>
> And that's exactly one of the issues of the current binding, the
> tolerance isn't stable, but rather per OPP.
>
> As an example look at two operating points of the i.MX5:
>
> /*  kHz min(uV) nom(uV) max(uV) */
>  166666  850000  900000 1400000
> 1200000 1300000 1350000 1400000
>
> So for the 1,2GHz OPP the voltage tolerance is just 3,7% as anything
> bigger would exceed the absolute maximum rating, but for the 166MHz OPP
> it is already 64,7%.
>
> I agree that I could just cap the rail at 1,4V and give a max voltage
> equal to that as max_volt to the regulator API, but actually the device
> doesn't even know it's tolerance as it has no max defined. It makes
> having a max_volt in the regulator API kind of senseless, as connected
> devices don't know their maximum rating.

Is'nt that(voltage-tolerance binding) a different problem that needs a
different solution rather than creating a new binding for OPP - which
actually makes no description sense (since the max is absolute max)?

How can we address that? should we add some logic that says:

if voltage tolerance == 0, then try min_uV to max_opp_uV?
OR
introduce a new binding that says absolute-max-voltage (this is what I
was planning on doing with voltage domain btw).. and ensure that that
is used?

Personally, I never liked voltage-tolerance binding precisely for the
same reason as you bring up.. but that is a ABI now and we need to see
if the requirements can match with it, or define a new one keeping the
old ABI alive..

Regards,
Nishanth Menon

Re: [RFC 2/2] PM / OPP: extend DT parsing to allow voltage ranges

[Lucas Stach]

Am Dienstag, den 20.05.2014, 11:09 -0500 schrieb Nishanth Menon:
> On Tue, May 20, 2014 at 10:48 AM, Lucas Stach <l.stach@pengutronix.de> wrote:
> > Am Dienstag, den 20.05.2014, 10:29 -0500 schrieb Nishanth Menon:
> >> On Tue, May 20, 2014 at 10:23 AM, Nishanth Menon <nm@ti.com> wrote:
> 
> [...]
> >> If you need to handle PMIC accuracy, use voltage-tolerance.
> >>
> > And that's exactly one of the issues of the current binding, the
> > tolerance isn't stable, but rather per OPP.
> >
> > As an example look at two operating points of the i.MX5:
> >
> > /*  kHz min(uV) nom(uV) max(uV) */
> >  166666  850000  900000 1400000
> > 1200000 1300000 1350000 1400000
> >
> > So for the 1,2GHz OPP the voltage tolerance is just 3,7% as anything
> > bigger would exceed the absolute maximum rating, but for the 166MHz OPP
> > it is already 64,7%.
> >
> > I agree that I could just cap the rail at 1,4V and give a max voltage
> > equal to that as max_volt to the regulator API, but actually the device
> > doesn't even know it's tolerance as it has no max defined. It makes
> > having a max_volt in the regulator API kind of senseless, as connected
> > devices don't know their maximum rating.
> 
> Is'nt that(voltage-tolerance binding) a different problem that needs a
> different solution rather than creating a new binding for OPP - which
> actually makes no description sense (since the max is absolute max)?
> 
Ok, I agree that having a nominal voltage isn't really required, so this
leaves us with min...max, where min is already there in the OPP
framework.

The question here is: are there any devices out there where we could
imagine a frequency dependent max-voltage? I could very well image such
a thing for CPU devices, where thermal constraints would force us to
reduce the regulator voltage to allow for a higher frequency. If any
device on the same rail needs a higher voltage this would prevent using
the higher OPP.

I have never seen such a device in practice, so maybe just having a
global max-voltage property is the right thing. I'll have to think about
this.

> How can we address that? should we add some logic that says:
> 
> if voltage tolerance == 0, then try min_uV to max_opp_uV?
> OR
> introduce a new binding that says absolute-max-voltage (this is what I
> was planning on doing with voltage domain btw).. and ensure that that
> is used?
> 
> Personally, I never liked voltage-tolerance binding precisely for the
> same reason as you bring up.. but that is a ABI now and we need to see
> if the requirements can match with it, or define a new one keeping the
> old ABI alive..
> 
I would like to keep the focus on new drivers using OPPs. How to fit in
drivers with existing bindings is to be defined later. I'm working on a
new cpufreq driver and would like to work out a solution that avoids
using this wobbly voltage-tolerance.

Regards,
Lucas
-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

Re: [RFC 2/2] PM / OPP: extend DT parsing to allow voltage ranges

[Nishanth Menon]

On Tue, May 20, 2014 at 11:45 AM, Lucas Stach <l.stach@pengutronix.de> wrote:
> Am Dienstag, den 20.05.2014, 11:09 -0500 schrieb Nishanth Menon:
>> On Tue, May 20, 2014 at 10:48 AM, Lucas Stach <l.stach@pengutronix.de> wrote:
>> > Am Dienstag, den 20.05.2014, 10:29 -0500 schrieb Nishanth Menon:
>> >> On Tue, May 20, 2014 at 10:23 AM, Nishanth Menon <nm@ti.com> wrote:
>>
>> [...]
>> >> If you need to handle PMIC accuracy, use voltage-tolerance.
>> >>
>> > And that's exactly one of the issues of the current binding, the
>> > tolerance isn't stable, but rather per OPP.
>> >
>> > As an example look at two operating points of the i.MX5:
>> >
>> > /*  kHz min(uV) nom(uV) max(uV) */
>> >  166666  850000  900000 1400000
>> > 1200000 1300000 1350000 1400000
>> >
>> > So for the 1,2GHz OPP the voltage tolerance is just 3,7% as anything
>> > bigger would exceed the absolute maximum rating, but for the 166MHz OPP
>> > it is already 64,7%.
>> >
>> > I agree that I could just cap the rail at 1,4V and give a max voltage
>> > equal to that as max_volt to the regulator API, but actually the device
>> > doesn't even know it's tolerance as it has no max defined. It makes
>> > having a max_volt in the regulator API kind of senseless, as connected
>> > devices don't know their maximum rating.
>>
>> Is'nt that(voltage-tolerance binding) a different problem that needs a
>> different solution rather than creating a new binding for OPP - which
>> actually makes no description sense (since the max is absolute max)?
>>
> Ok, I agree that having a nominal voltage isn't really required, so this
> leaves us with min...max, where min is already there in the OPP
> framework.
>
> The question here is: are there any devices out there where we could
> imagine a frequency dependent max-voltage? I could very well image such
> a thing for CPU devices, where thermal constraints would force us to
> reduce the regulator voltage to allow for a higher frequency. If any
> device on the same rail needs a higher voltage this would prevent using
> the higher OPP.
>
> I have never seen such a device in practice, so maybe just having a
> global max-voltage property is the right thing. I'll have to think about
> this.

That creates a new set of problem -> precisely because you will have
to handle that as part of DVFS sequencing to such an OPP. Further, for
lower OPP, if voltage X works for frequency F1, why would voltage X1>X
not work for frequency Y?

As you indicated, So far, most devices we have heard of adheres to
this - this is precisely why I was curious on your patch series. From
our discussion so far (thanks for your patience), we dont have such a
need here as well. Ideally if PMIC constraints cannot achieve that
voltage, regulator framework will return back to us with error and
handling is done - we have that happening from to time on customer
platforms with TI chips + certain PMICs - there are different optional
solutions for it (OPP modifier[1] series tried to address that
generically for TI and iMx6 - but got killed)..

For the "over clocked" devices, we have boost-frequencies that are
being debated upon. Those seem to have been captured in the separate
discussion already.

>
>> How can we address that? should we add some logic that says:
>>
>> if voltage tolerance == 0, then try min_uV to max_opp_uV?
>> OR
>> introduce a new binding that says absolute-max-voltage (this is what I
>> was planning on doing with voltage domain btw).. and ensure that that
>> is used?
>>
>> Personally, I never liked voltage-tolerance binding precisely for the
>> same reason as you bring up.. but that is a ABI now and we need to see
>> if the requirements can match with it, or define a new one keeping the
>> old ABI alive..
>>
> I would like to keep the focus on new drivers using OPPs. How to fit in
> drivers with existing bindings is to be defined later. I'm working on a
> new cpufreq driver and would like to work out a solution that avoids
> using this wobbly voltage-tolerance.


Agreed, would love to see a proposal if something could be done with
this "voltage-tolerance" without loosing it's current meaning (without
breaking ABI - maybe we could deprecate it at a later point in time -
but I doubt that - older dtb argument :( ).

Meanwhile, based on our discussion so far, I'd say a NAK for the
moment for the current series.

[1] http://comments.gmane.org/gmane.linux.ports.arm.kernel/309466

Regards,
Nishanth Menon

Re: [RFC 2/2] PM / OPP: extend DT parsing to allow voltage ranges

[Lucas Stach]

+CC: Mark Brown

Hi Mark,

we are having a discussion here about the interactions between the OPP
and the regulator framework and I would like to get your input on this
matter.

To summarize things: I'm working on a new cpufreq driver and I think the
current practice of using a "voltage-tolerance" is bogus as most devices
don't actually have a fixed tolerance but rather an absolute maximum
rating which should not be exceeded.

The voltage tolerance is used as the regulator needs to be given a
sensible range for set_voltage(). The minimum voltage is defined by the
current OPP, maximum not so much. Now most cpufreq drivers just add a
tolerance to the minimum to get the max value and call it a day, but
actually I think it would be more sensible to stick the absolute maximum
rating in there. The problem is a device driver currently doesn't know
about the maximum rating of it's device.

On the other hand Nishanth argues that regulator constraints should be
set correctly to limit the voltage to the absolute maximum rating and I
can see his point here. But only using regulator constraints would make
the max voltage parameter in set_voltage() superfluous and we could just
set it to INT_MAX, which feels really wrong.

So with the realization that the maximum rating isn't really dependent
on the current OPP it seems we could just have a generic
"*-supply-max-voltage" property to describe a reasonable upper bound
instead of this wobbly voltage-tolerance. Does this sound sensible to
you?

Regards,
Lucas

Am Dienstag, den 20.05.2014, 12:02 -0500 schrieb Nishanth Menon:
> On Tue, May 20, 2014 at 11:45 AM, Lucas Stach <l.stach@pengutronix.de> wrote:
> > Am Dienstag, den 20.05.2014, 11:09 -0500 schrieb Nishanth Menon:
> >> On Tue, May 20, 2014 at 10:48 AM, Lucas Stach <l.stach@pengutronix.de> wrote:
> >> > Am Dienstag, den 20.05.2014, 10:29 -0500 schrieb Nishanth Menon:
> >> >> On Tue, May 20, 2014 at 10:23 AM, Nishanth Menon <nm@ti.com> wrote:
> >>
> >> [...]
> >> >> If you need to handle PMIC accuracy, use voltage-tolerance.
> >> >>
> >> > And that's exactly one of the issues of the current binding, the
> >> > tolerance isn't stable, but rather per OPP.
> >> >
> >> > As an example look at two operating points of the i.MX5:
> >> >
> >> > /*  kHz min(uV) nom(uV) max(uV) */
> >> >  166666  850000  900000 1400000
> >> > 1200000 1300000 1350000 1400000
> >> >
> >> > So for the 1,2GHz OPP the voltage tolerance is just 3,7% as anything
> >> > bigger would exceed the absolute maximum rating, but for the 166MHz OPP
> >> > it is already 64,7%.
> >> >
> >> > I agree that I could just cap the rail at 1,4V and give a max voltage
> >> > equal to that as max_volt to the regulator API, but actually the device
> >> > doesn't even know it's tolerance as it has no max defined. It makes
> >> > having a max_volt in the regulator API kind of senseless, as connected
> >> > devices don't know their maximum rating.
> >>
> >> Is'nt that(voltage-tolerance binding) a different problem that needs a
> >> different solution rather than creating a new binding for OPP - which
> >> actually makes no description sense (since the max is absolute max)?
> >>
> > Ok, I agree that having a nominal voltage isn't really required, so this
> > leaves us with min...max, where min is already there in the OPP
> > framework.
> >
> > The question here is: are there any devices out there where we could
> > imagine a frequency dependent max-voltage? I could very well image such
> > a thing for CPU devices, where thermal constraints would force us to
> > reduce the regulator voltage to allow for a higher frequency. If any
> > device on the same rail needs a higher voltage this would prevent using
> > the higher OPP.
> >
> > I have never seen such a device in practice, so maybe just having a
> > global max-voltage property is the right thing. I'll have to think about
> > this.
> 
> That creates a new set of problem -> precisely because you will have
> to handle that as part of DVFS sequencing to such an OPP. Further, for
> lower OPP, if voltage X works for frequency F1, why would voltage X1>X
> not work for frequency Y?
> 
[...]
> 
> >
> >> How can we address that? should we add some logic that says:
> >>
> >> if voltage tolerance == 0, then try min_uV to max_opp_uV?
> >> OR
> >> introduce a new binding that says absolute-max-voltage (this is what I
> >> was planning on doing with voltage domain btw).. and ensure that that
> >> is used?
> >>
> >> Personally, I never liked voltage-tolerance binding precisely for the
> >> same reason as you bring up.. but that is a ABI now and we need to see
> >> if the requirements can match with it, or define a new one keeping the
> >> old ABI alive..
> >>
> > I would like to keep the focus on new drivers using OPPs. How to fit in
> > drivers with existing bindings is to be defined later. I'm working on a
> > new cpufreq driver and would like to work out a solution that avoids
> > using this wobbly voltage-tolerance.
> 
> 
> Agreed, would love to see a proposal if something could be done with
> this "voltage-tolerance" without loosing it's current meaning (without
> breaking ABI - maybe we could deprecate it at a later point in time -
> but I doubt that - older dtb argument :( ).
> 
> Meanwhile, based on our discussion so far, I'd say a NAK for the
> moment for the current series.
> 
> [1] http://comments.gmane.org/gmane.linux.ports.arm.kernel/309466
> 
> Regards,
> Nishanth Menon

-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

Re: [RFC 1/2] PM / OPP: allow to use voltage ranges

[Pavel Machek]

On Tue 2014-05-20 16:27:39, Lucas Stach wrote:
> In many cases we don't have a strict one to one relation
> between a frequency and a voltage, but rather an operating
> frequency plus a voltage range that need to be maintained
> for this one frequency.
> 
> This patch does not change any behavior, but only expands
> the API to cope with voltage ranges.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>

Reviewed-by: Pavel Machek <pavel@ucw.cz>

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFC 2/2] PM / OPP: extend DT parsing to allow voltage ranges

[Pavel Machek]

On Tue 2014-05-20 16:27:40, Lucas Stach wrote:
> Following the introduction of voltage ranges into OPP
> we need a way to encode them in the device tree in a
> similar fashion to the non-ranged versions.
> 
> To keep compatibility with old DTs the parsing function
> is changed to understand both versions.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>

Reviewed-by: Pavel Machek <pavel@ucw.cz>

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFC 2/2] PM / OPP: extend DT parsing to allow voltage ranges

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 3071 bytes --]

On Wed, May 21, 2014 at 11:47:14AM +0200, Lucas Stach wrote:

> To summarize things: I'm working on a new cpufreq driver and I think the
> current practice of using a "voltage-tolerance" is bogus as most devices
> don't actually have a fixed tolerance but rather an absolute maximum
> rating which should not be exceeded.

We actually need both options here but yes, I agree.

> The voltage tolerance is used as the regulator needs to be given a
> sensible range for set_voltage(). The minimum voltage is defined by the
> current OPP, maximum not so much. Now most cpufreq drivers just add a
> tolerance to the minimum to get the max value and call it a day, but
> actually I think it would be more sensible to stick the absolute maximum
> rating in there. The problem is a device driver currently doesn't know
> about the maximum rating of it's device.

This depends very much on how the chip is specified - as far as I can
tell some manufacturers specify things really tightly and only guarantee
that the chip will operate at specific operating points while others do
the more expected thing and specify a minimum voltage for the OPP and a
maximum rated voltage with any voltage in the range being in spec.  I
expect this is due to the need to guarantee performance so they're only
quoting what's been fully simulated, verified and characterised in the
datasheet.

Realistically it's probably unlikely that there would be any practical
problems other than excessive power consumption going over voltage so
long as it is within the maximum rated operational voltage but equally
well if the datasheet specifies something more restrictive it seems
sensible to follow what the datasheet said just in case there's some
good reason for it (and to make it easier to get support from the
hardware people!).

The people who did that code originally had the first case but we should
as you say be able to handle the second case and allow each OPP (or
possibly just the device as a whole) to have a ceiling put in place.

> On the other hand Nishanth argues that regulator constraints should be
> set correctly to limit the voltage to the absolute maximum rating and I
> can see his point here. But only using regulator constraints would make
> the max voltage parameter in set_voltage() superfluous and we could just
> set it to INT_MAX, which feels really wrong.

That would work as well of course but like you say eew.  :)

> So with the realization that the maximum rating isn't really dependent
> on the current OPP it seems we could just have a generic
> "*-supply-max-voltage" property to describe a reasonable upper bound
> instead of this wobbly voltage-tolerance. Does this sound sensible to
> you?

Yes, I definitely think that option should exist.  I don't know if it's
worth doing it per OPP rather than for the chip as a whole in case
that's a thing people want to use but I'd be surprised if anyone
noticed.  We could also infer the maximum voltage from the highest
voltage specified for an OPP if some boolean property was set which
might be a bit less redundant.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC 2/2] PM / OPP: extend DT parsing to allow voltage ranges

[Lucas Stach]

Am Donnerstag, den 22.05.2014, 00:33 +0100 schrieb Mark Brown:
> On Wed, May 21, 2014 at 11:47:14AM +0200, Lucas Stach wrote:
> 
> > To summarize things: I'm working on a new cpufreq driver and I think the
> > current practice of using a "voltage-tolerance" is bogus as most devices
> > don't actually have a fixed tolerance but rather an absolute maximum
> > rating which should not be exceeded.
> 
> We actually need both options here but yes, I agree.
> 
> > The voltage tolerance is used as the regulator needs to be given a
> > sensible range for set_voltage(). The minimum voltage is defined by the
> > current OPP, maximum not so much. Now most cpufreq drivers just add a
> > tolerance to the minimum to get the max value and call it a day, but
> > actually I think it would be more sensible to stick the absolute maximum
> > rating in there. The problem is a device driver currently doesn't know
> > about the maximum rating of it's device.
> 
> This depends very much on how the chip is specified - as far as I can
> tell some manufacturers specify things really tightly and only guarantee
> that the chip will operate at specific operating points while others do
> the more expected thing and specify a minimum voltage for the OPP and a
> maximum rated voltage with any voltage in the range being in spec.  I
> expect this is due to the need to guarantee performance so they're only
> quoting what's been fully simulated, verified and characterised in the
> datasheet.
> 
> Realistically it's probably unlikely that there would be any practical
> problems other than excessive power consumption going over voltage so
> long as it is within the maximum rated operational voltage but equally
> well if the datasheet specifies something more restrictive it seems
> sensible to follow what the datasheet said just in case there's some
> good reason for it (and to make it easier to get support from the
> hardware people!).
> 
> The people who did that code originally had the first case but we should
> as you say be able to handle the second case and allow each OPP (or
> possibly just the device as a whole) to have a ceiling put in place.
> 
> > On the other hand Nishanth argues that regulator constraints should be
> > set correctly to limit the voltage to the absolute maximum rating and I
> > can see his point here. But only using regulator constraints would make
> > the max voltage parameter in set_voltage() superfluous and we could just
> > set it to INT_MAX, which feels really wrong.
> 
> That would work as well of course but like you say eew.  :)
> 
> > So with the realization that the maximum rating isn't really dependent
> > on the current OPP it seems we could just have a generic
> > "*-supply-max-voltage" property to describe a reasonable upper bound
> > instead of this wobbly voltage-tolerance. Does this sound sensible to
> > you?
> 
> Yes, I definitely think that option should exist.  I don't know if it's
> worth doing it per OPP rather than for the chip as a whole in case
> that's a thing people want to use but I'd be surprised if anyone
> noticed.  We could also infer the maximum voltage from the highest
> voltage specified for an OPP if some boolean property was set which
> might be a bit less redundant.

Thanks for the feedback.

I think I'll go for the ceiling per device option, as I'm not really
able to convince myself anymore that a ceiling per OPP is necessary.

This shifts things from a change to the OPP framework to a change to the
regulator framework, as I would like to add this property to the device
supply definition. Patches upcoming.

I would really like to have a clear absolute maximum per device, as this
would allow me to infer the voltage tolerance of the device from the
difference between the highest OPP and maximum voltage.

Regards,
Lucas
-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

Re: [RFC 2/2] PM / OPP: extend DT parsing to allow voltage ranges

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1648 bytes --]

On Thu, May 22, 2014 at 12:24:58PM +0200, Lucas Stach wrote:

> This shifts things from a change to the OPP framework to a change to the
> regulator framework, as I would like to add this property to the device
> supply definition. Patches upcoming.

In the regulator framework this seems completely redundant - we already
have the maximum voltage specifiable, it's hard to see a case where
you'd want a maximum that wasn't the maximum for the board.  I'd expect
the cpufreq code to just read the existing property.

This disadvantage of doing this without any explicit configuration on
the cpufreq side is that it means we start ignoring the OPP constraints
of CPUs that aren't specified to allow that.  Perhaps we don't care
though.

> I would really like to have a clear absolute maximum per device, as this
> would allow me to infer the voltage tolerance of the device from the
> difference between the highest OPP and maximum voltage.

I don't think this is a good way of specifying this information.  I
think if we want to start paying attention to the tolerances in that way
(which we really ought to do) we should be actually teaching the
framework about them - for example have the regulator driver able to
specify the regulation accuracy so set_voltage() can guarantee that the
constraint is met and probably also have a way of specifying tolerances
on the ranged set_voltage() too.

Trying to infer this information from absolute numbers seems likely to
get us into trouble at some point, both in terms of being harder for
people to use to get the result they want and in terms of not being able
to convey the information needed at all.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC 2/2] PM / OPP: extend DT parsing to allow voltage ranges

[Nishanth Menon]

On 05/22/2014 12:58 PM, Mark Brown wrote:
> On Thu, May 22, 2014 at 12:24:58PM +0200, Lucas Stach wrote:
> 
>> This shifts things from a change to the OPP framework to a change to the
>> regulator framework, as I would like to add this property to the device
>> supply definition. Patches upcoming.
> 
> In the regulator framework this seems completely redundant - we already
> have the maximum voltage specifiable, it's hard to see a case where
> you'd want a maximum that wasn't the maximum for the board.  I'd expect
> the cpufreq code to just read the existing property.
> 
> This disadvantage of doing this without any explicit configuration on
> the cpufreq side is that it means we start ignoring the OPP constraints
> of CPUs that aren't specified to allow that.  Perhaps we don't care
> though.
> 
>> I would really like to have a clear absolute maximum per device, as this
>> would allow me to infer the voltage tolerance of the device from the
>> difference between the highest OPP and maximum voltage.
> 
> I don't think this is a good way of specifying this information.  I
> think if we want to start paying attention to the tolerances in that way
> (which we really ought to do) we should be actually teaching the
> framework about them - for example have the regulator driver able to
> specify the regulation accuracy so set_voltage() can guarantee that the
> constraint is met and probably also have a way of specifying tolerances
> on the ranged set_voltage() too.
> 
> Trying to infer this information from absolute numbers seems likely to
> get us into trouble at some point, both in terms of being harder for
> people to use to get the result they want and in terms of not being able
> to convey the information needed at all.
> 
Apologies on coming back to this discussion so late.

Digging through older emails on this topic, there is one more factor I
had forgotten - IR drop. Just to summarize the factors I can think of
for voltage:
a) OPP min operational voltage (specified in SoC data sheet) - covered
in operating-points dt description
b) device max operational voltage (specified in SoC data sheet) - this
is what we are discussing - I'd go with a max-operational-voltage dt
property to describe this. I see some old threads like [1] which tried
something similar.
c) certain devices allowing voltage tolerance ranges (original
definition of voltage-tolerance that I can find)

d) PMIC min/max voltage range - (specified in PMIC data sheet) - we
already have constraints description ability in DT today.
e) PMIC accuracy - voltage X set on the PMIC might depend on
peak-to-peak noise and after caps to cleanup might end up be +- y% of
attempted voltage X and may actually even vary (example - depending on
PMIC and current drawn, PWM to PFM mode transitions might result in
variant voltages). I think voltage-tolerance was used to compensate
here as well in certain instances?

f) based on board design, the voltage X set on PMIC and at PMIC
ball(after accounting for {e}), eventually results in Voltage Y at the
SoC ball where X-Y is the IR drop. I see some instances (like AM335x)
using voltage-tolerance to account for that as well. - this is
primarily a board factor. Some PMICs like TWL/TPS series have feedback
loop to account for this to save s/w from figuring this out, but not
all PMICs have that ability and some boards may not have wired in the
feedback loop properly.. Again, to complicate life, the current factor
varies depending on type of activity and type of device involved -
without proper characterization, this tends to be kinda hard to quantify.


[1]
http://www.gossamer-threads.com/lists/linux/kernel/1811056?do=post_view_threaded

-- 
Regards,
Nishanth Menon