linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [RFC 0/5] Get voltage from parent if not available on child
@ 2014-07-29 16:28 Javier Martinez Canillas
  2014-07-29 16:28 ` [RFC 1/5] regulator: core: Get voltage from parent if not available Javier Martinez Canillas
                   ` (4 more replies)
  0 siblings, 5 replies; 16+ messages in thread
From: Javier Martinez Canillas @ 2014-07-29 16:28 UTC (permalink / raw)
  To: linux-arm-kernel

Hello,

The motivation for this series is that some regulators are
really just switches that provide as output voltage what is
supplied to them as input voltage. These components can be
only switched on and off so the drivers usually implements
a subset of the possible regulator operations. That is only
.enable, .disable and .is_enabled but no .{get,list}_voltage

But there is code that assume all regulators can provide
their output voltage so if one of these load switches are
used as an input supply a call to regulator_get_voltage() or
regulator_list_voltage() will fail.

As an example in [0] mmc_regulator_get_supply() is used to
obtain the vmmc supply but on Peach Pit and Pi boards this
is tps65090 fet4 which is a load switch so the call to
mmc_regulator_get_ocrmask() fails since it tries to obtain
the regulator voltage count and list.

mmc_regulator_set_ocr() function fails as well since it calls
regulator_get_voltage() which the driver does not implement.

Since this seems to be a general issue we can pass through
and query the parent supply voltage when a child regulator
does not implement the handlers to obtain its output voltage.

The series have been tested on a Exynos 5420 Peach Pit board
and is composed of the following patches: 

Javier Martinez Canillas (5):
  regulator: core: Get voltage from parent if not available
  regulator: core: Allow to get voltage count and list from parent
  regulator: core: Only apply constraints if available on list voltage
  regulator: tps65090: Set voltage for fixed regulators
  ARM: dts: Improve Peach Pit and Pi power scheme model

 arch/arm/boot/dts/exynos5420-peach-pit.dts | 12 +++++------
 arch/arm/boot/dts/exynos5800-peach-pi.dts  | 12 +++++------
 drivers/regulator/core.c                   | 31 ++++++++++++++++++++-------
 drivers/regulator/tps65090-regulator.c     | 34 ++++++++++++++++++------------
 4 files changed, 56 insertions(+), 33 deletions(-)

Patch 1 adds support to get the voltage of the parent supply
while patch 2 does the same for selector count and selector.
Patch 3 only applies the constraints when querying a selector
if there were constraints defined for this regulator. Otherwise
the constraints of the parent supply will be applied when
calling regulator_list_voltage() for the parent supply. Patch
4 adds information about the voltage output for the fixed
regulators found on the tps65090 since these are parent supply
of the fets load switches. Finally patch 5 models the real
relation between regulators in the Peach Pit and Pi boards
instead of the simplistic model that is implemented now.
This last patch should not be merged with the series but since
is a RFC I included so it can be tested on Peach boards and
to provide the complete picture.

Best regards,
Javier

[0]: https://patchwork.kernel.org/patch/4401231/
[1]: http://lxr.free-electrons.com/source/drivers/mmc/core/core.c#L1253

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

* [RFC 1/5] regulator: core: Get voltage from parent if not available
  2014-07-29 16:28 [RFC 0/5] Get voltage from parent if not available on child Javier Martinez Canillas
@ 2014-07-29 16:28 ` Javier Martinez Canillas
  2014-07-29 18:26   ` Mark Brown
  2014-07-29 16:28 ` [RFC 2/5] regulator: core: Allow to get voltage count and list from parent Javier Martinez Canillas
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 16+ messages in thread
From: Javier Martinez Canillas @ 2014-07-29 16:28 UTC (permalink / raw)
  To: linux-arm-kernel

Load switches are modeled as regulators but they just provide
the voltage of their parent input supply. So the drivers for
these switches usually don't provide a .get_voltage function
handler but there is code in the kernel that assumes that all
regulators should be able to provide its current voltage rail.

So, if the output voltage for a regulator is not available and
it has a parent supply, then pass the voltage of its parent.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 drivers/regulator/core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 69c9c08..089cea8 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2695,6 +2695,8 @@ static int _regulator_get_voltage(struct regulator_dev *rdev)
 		ret = rdev->desc->ops->list_voltage(rdev, 0);
 	} else if (rdev->desc->fixed_uV && (rdev->desc->n_voltages == 1)) {
 		ret = rdev->desc->fixed_uV;
+	} else if (rdev->supply) {
+		ret = regulator_get_voltage(rdev->supply);
 	} else {
 		return -EINVAL;
 	}
-- 
2.0.0.rc2

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

* [RFC 2/5] regulator: core: Allow to get voltage count and list from parent
  2014-07-29 16:28 [RFC 0/5] Get voltage from parent if not available on child Javier Martinez Canillas
  2014-07-29 16:28 ` [RFC 1/5] regulator: core: Get voltage from parent if not available Javier Martinez Canillas
@ 2014-07-29 16:28 ` Javier Martinez Canillas
  2014-07-29 18:25   ` Mark Brown
  2014-07-29 16:28 ` [RFC 3/5] regulator: core: Only apply constraints if available on list voltage Javier Martinez Canillas
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 16+ messages in thread
From: Javier Martinez Canillas @ 2014-07-29 16:28 UTC (permalink / raw)
  To: linux-arm-kernel

Load switches are modeled as regulators but they just provide
the voltage of their parent input supply. So, the drivers for
these switches usually neither provide a .list_voltage handler
not set a .n_voltages count. But there is code in the kernel
that assumes that all regulators should be able to provide this
information (e.g: cpufreq and mmc subsystems).

If the voltage count and list are not available for a regulator
and it has a parent input supply, then use the parent values.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 drivers/regulator/core.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 089cea8..a3c3785 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2186,7 +2186,13 @@ int regulator_count_voltages(struct regulator *regulator)
 {
 	struct regulator_dev	*rdev = regulator->rdev;
 
-	return rdev->desc->n_voltages ? : -EINVAL;
+	if (rdev->desc->n_voltages)
+		return rdev->desc->n_voltages;
+
+	if (!rdev->supply)
+		return -EINVAL;
+
+	return regulator_count_voltages(rdev->supply);
 }
 EXPORT_SYMBOL_GPL(regulator_count_voltages);
 
@@ -2209,12 +2215,17 @@ int regulator_list_voltage(struct regulator *regulator, unsigned selector)
 	if (rdev->desc->fixed_uV && rdev->desc->n_voltages == 1 && !selector)
 		return rdev->desc->fixed_uV;
 
-	if (!ops->list_voltage || selector >= rdev->desc->n_voltages)
+	if (ops->list_voltage) {
+		if (selector >= rdev->desc->n_voltages)
+			return -EINVAL;
+		mutex_lock(&rdev->mutex);
+		ret = ops->list_voltage(rdev, selector);
+		mutex_unlock(&rdev->mutex);
+	} else if (rdev->supply) {
+		ret = regulator_list_voltage(rdev->supply, selector);
+	} else {
 		return -EINVAL;
-
-	mutex_lock(&rdev->mutex);
-	ret = ops->list_voltage(rdev, selector);
-	mutex_unlock(&rdev->mutex);
+	}
 
 	if (ret > 0) {
 		if (ret < rdev->constraints->min_uV)
-- 
2.0.0.rc2

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

* [RFC 3/5] regulator: core: Only apply constraints if available on list voltage
  2014-07-29 16:28 [RFC 0/5] Get voltage from parent if not available on child Javier Martinez Canillas
  2014-07-29 16:28 ` [RFC 1/5] regulator: core: Get voltage from parent if not available Javier Martinez Canillas
  2014-07-29 16:28 ` [RFC 2/5] regulator: core: Allow to get voltage count and list from parent Javier Martinez Canillas
@ 2014-07-29 16:28 ` Javier Martinez Canillas
  2014-07-29 17:18   ` Mark Brown
  2014-07-29 16:28 ` [RFC 4/5] regulator: tps65090: Set voltage for fixed regulators Javier Martinez Canillas
  2014-07-29 16:28 ` [RFC 5/5] ARM: dts: Improve Peach Pit and Pi power scheme model Javier Martinez Canillas
  4 siblings, 1 reply; 16+ messages in thread
From: Javier Martinez Canillas @ 2014-07-29 16:28 UTC (permalink / raw)
  To: linux-arm-kernel

If a selector can't be used on a platform due to voltage constraints,
regulator_list_voltage() returns 0. Doing this unconditionally made
sense since constraints were set in machine_constraints_voltage() at
regulator registration time.

But for load switches that don't define a voltage output, the parent
supply voltage is used so the constraints should only be applied if
they were defined for the child regulators.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 drivers/regulator/core.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index a3c3785..7472535 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2228,9 +2228,11 @@ int regulator_list_voltage(struct regulator *regulator, unsigned selector)
 	}
 
 	if (ret > 0) {
-		if (ret < rdev->constraints->min_uV)
+		if (rdev->constraints->min_uV &&
+		    ret < rdev->constraints->min_uV)
 			ret = 0;
-		else if (ret > rdev->constraints->max_uV)
+		else if (rdev->constraints->max_uV &&
+			 ret > rdev->constraints->max_uV)
 			ret = 0;
 	}
 
-- 
2.0.0.rc2

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

* [RFC 4/5] regulator: tps65090: Set voltage for fixed regulators
  2014-07-29 16:28 [RFC 0/5] Get voltage from parent if not available on child Javier Martinez Canillas
                   ` (2 preceding siblings ...)
  2014-07-29 16:28 ` [RFC 3/5] regulator: core: Only apply constraints if available on list voltage Javier Martinez Canillas
@ 2014-07-29 16:28 ` Javier Martinez Canillas
  2014-07-29 17:36   ` Mark Brown
  2014-07-29 16:28 ` [RFC 5/5] ARM: dts: Improve Peach Pit and Pi power scheme model Javier Martinez Canillas
  4 siblings, 1 reply; 16+ messages in thread
From: Javier Martinez Canillas @ 2014-07-29 16:28 UTC (permalink / raw)
  To: linux-arm-kernel

According to the tps65090 data manual [0], the DCDC1 and DCDC2
step-down converters and the LDO's have a fixed output voltage.

Add this information to the driver since these fixed regulators
be used as input supply for load switches which don't provide
an output voltage and their parent supply voltage is used.

[0]: http://www.ti.com/lit/gpn/tps65090

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 drivers/regulator/tps65090-regulator.c | 34 +++++++++++++++++++++-------------
 1 file changed, 21 insertions(+), 13 deletions(-)

diff --git a/drivers/regulator/tps65090-regulator.c b/drivers/regulator/tps65090-regulator.c
index 2064b3f..919f5ce 100644
--- a/drivers/regulator/tps65090-regulator.c
+++ b/drivers/regulator/tps65090-regulator.c
@@ -192,12 +192,14 @@ static struct regulator_ops tps65090_fet_control_ops = {
 static struct regulator_ops tps65090_ldo_ops = {
 };
 
-#define tps65090_REG_DESC(_id, _sname, _en_reg, _en_bits, _ops)	\
+#define tps65090_REG_DESC(_id, _sname, _en_reg, _en_bits, _nvolt, _volt, _ops) \
 {							\
 	.name = "TPS65090_RAILS"#_id,			\
 	.supply_name = _sname,				\
 	.id = TPS65090_REGULATOR_##_id,			\
+	.n_voltages = _nvolt,				\
 	.ops = &_ops,					\
+	.fixed_uV = _volt,				\
 	.enable_reg = _en_reg,				\
 	.enable_val = _en_bits,				\
 	.enable_mask = _en_bits,			\
@@ -205,39 +207,45 @@ static struct regulator_ops tps65090_ldo_ops = {
 	.owner = THIS_MODULE,				\
 }
 
+#define tps65090_REG_FIX(_id, _sname, en_reg, _en_bits, _volt, _ops) \
+	tps65090_REG_DESC(_id, _sname, en_reg, _en_bits, 1, _volt, _ops)
+
+#define tps65090_REG_VAR(_id, _sname, en_reg, _en_bits, _ops) \
+	tps65090_REG_DESC(_id, _sname, en_reg, _en_bits, 0, 0, _ops)
+
 static struct regulator_desc tps65090_regulator_desc[] = {
-	tps65090_REG_DESC(DCDC1, "vsys1",   0x0C, BIT(CTRL_EN_BIT),
+	tps65090_REG_FIX(DCDC1, "vsys1",   0x0C, BIT(CTRL_EN_BIT), 5000000,
 			  tps65090_reg_control_ops),
-	tps65090_REG_DESC(DCDC2, "vsys2",   0x0D, BIT(CTRL_EN_BIT),
+	tps65090_REG_FIX(DCDC2, "vsys2",   0x0D, BIT(CTRL_EN_BIT), 3300000,
 			  tps65090_reg_control_ops),
-	tps65090_REG_DESC(DCDC3, "vsys3",   0x0E, BIT(CTRL_EN_BIT),
+	tps65090_REG_VAR(DCDC3, "vsys3",   0x0E, BIT(CTRL_EN_BIT),
 			  tps65090_reg_control_ops),
 
-	tps65090_REG_DESC(FET1,  "infet1",  0x0F,
+	tps65090_REG_VAR(FET1,  "infet1",  0x0F,
 			  BIT(CTRL_EN_BIT) | BIT(CTRL_PG_BIT),
 			  tps65090_fet_control_ops),
-	tps65090_REG_DESC(FET2,  "infet2",  0x10,
+	tps65090_REG_VAR(FET2,  "infet2",  0x10,
 			  BIT(CTRL_EN_BIT) | BIT(CTRL_PG_BIT),
 			  tps65090_fet_control_ops),
-	tps65090_REG_DESC(FET3,  "infet3",  0x11,
+	tps65090_REG_VAR(FET3,  "infet3",  0x11,
 			  BIT(CTRL_EN_BIT) | BIT(CTRL_PG_BIT),
 			  tps65090_fet_control_ops),
-	tps65090_REG_DESC(FET4,  "infet4",  0x12,
+	tps65090_REG_VAR(FET4,  "infet4",  0x12,
 			  BIT(CTRL_EN_BIT) | BIT(CTRL_PG_BIT),
 			  tps65090_fet_control_ops),
-	tps65090_REG_DESC(FET5,  "infet5",  0x13,
+	tps65090_REG_VAR(FET5,  "infet5",  0x13,
 			  BIT(CTRL_EN_BIT) | BIT(CTRL_PG_BIT),
 			  tps65090_fet_control_ops),
-	tps65090_REG_DESC(FET6,  "infet6",  0x14,
+	tps65090_REG_VAR(FET6,  "infet6",  0x14,
 			  BIT(CTRL_EN_BIT) | BIT(CTRL_PG_BIT),
 			  tps65090_fet_control_ops),
-	tps65090_REG_DESC(FET7,  "infet7",  0x15,
+	tps65090_REG_VAR(FET7,  "infet7",  0x15,
 			  BIT(CTRL_EN_BIT) | BIT(CTRL_PG_BIT),
 			  tps65090_fet_control_ops),
 
-	tps65090_REG_DESC(LDO1,  "vsys-l1", 0, 0,
+	tps65090_REG_FIX(LDO1,  "vsys-l1", 0, 0, 5000000,
 			  tps65090_ldo_ops),
-	tps65090_REG_DESC(LDO2,  "vsys-l2", 0, 0,
+	tps65090_REG_FIX(LDO2,  "vsys-l2", 0, 0, 3300000,
 			  tps65090_ldo_ops),
 };
 
-- 
2.0.0.rc2

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

* [RFC 5/5] ARM: dts: Improve Peach Pit and Pi power scheme model
  2014-07-29 16:28 [RFC 0/5] Get voltage from parent if not available on child Javier Martinez Canillas
                   ` (3 preceding siblings ...)
  2014-07-29 16:28 ` [RFC 4/5] regulator: tps65090: Set voltage for fixed regulators Javier Martinez Canillas
@ 2014-07-29 16:28 ` Javier Martinez Canillas
  2014-07-29 17:20   ` Mark Brown
  4 siblings, 1 reply; 16+ messages in thread
From: Javier Martinez Canillas @ 2014-07-29 16:28 UTC (permalink / raw)
  To: linux-arm-kernel

The DeviceTree files for the Peach Pit and Pi machines have
a simplistic model of the connections between the different
regulators since not all the tps65090 regulators get their
input supply voltage from the VDC. DCDC1-3, LD0-1 and fet7
parent supply is indded VDC but the fet1-6 get their input
supply from the DCDC1 and DCDC2 output voltage rails.

This information has to be present in the DTS since fets
are switches that don't provide an output voltage so when
this is needed, the parent input supply voltage is queried.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 arch/arm/boot/dts/exynos5420-peach-pit.dts | 12 ++++++------
 arch/arm/boot/dts/exynos5800-peach-pi.dts  | 12 ++++++------
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/arch/arm/boot/dts/exynos5420-peach-pit.dts b/arch/arm/boot/dts/exynos5420-peach-pit.dts
index 7656a42..acc2936 100644
--- a/arch/arm/boot/dts/exynos5420-peach-pit.dts
+++ b/arch/arm/boot/dts/exynos5420-peach-pit.dts
@@ -367,12 +367,12 @@
 				vsys2-supply = <&vbat>;
 				vsys3-supply = <&vbat>;
 				infet1-supply = <&vbat>;
-				infet2-supply = <&vbat>;
-				infet3-supply = <&vbat>;
-				infet4-supply = <&vbat>;
-				infet5-supply = <&vbat>;
-				infet6-supply = <&vbat>;
-				infet7-supply = <&vbat>;
+				infet2-supply = <&tps65090_dcdc1>;
+				infet3-supply = <&tps65090_dcdc2>;
+				infet4-supply = <&tps65090_dcdc2>;
+				infet5-supply = <&tps65090_dcdc2>;
+				infet6-supply = <&tps65090_dcdc2>;
+				infet7-supply = <&tps65090_dcdc1>;
 				vsys-l1-supply = <&vbat>;
 				vsys-l2-supply = <&vbat>;
 
diff --git a/arch/arm/boot/dts/exynos5800-peach-pi.dts b/arch/arm/boot/dts/exynos5800-peach-pi.dts
index 5f3e54f..6b83b98 100644
--- a/arch/arm/boot/dts/exynos5800-peach-pi.dts
+++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts
@@ -365,12 +365,12 @@
 				vsys2-supply = <&vbat>;
 				vsys3-supply = <&vbat>;
 				infet1-supply = <&vbat>;
-				infet2-supply = <&vbat>;
-				infet3-supply = <&vbat>;
-				infet4-supply = <&vbat>;
-				infet5-supply = <&vbat>;
-				infet6-supply = <&vbat>;
-				infet7-supply = <&vbat>;
+				infet2-supply = <&tps65090_dcdc1>;
+				infet3-supply = <&tps65090_dcdc2>;
+				infet4-supply = <&tps65090_dcdc2>;
+				infet5-supply = <&tps65090_dcdc2>;
+				infet6-supply = <&tps65090_dcdc2>;
+				infet7-supply = <&tps65090_dcdc1>;
 				vsys-l1-supply = <&vbat>;
 				vsys-l2-supply = <&vbat>;
 
-- 
2.0.0.rc2

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

* [RFC 3/5] regulator: core: Only apply constraints if available on list voltage
  2014-07-29 16:28 ` [RFC 3/5] regulator: core: Only apply constraints if available on list voltage Javier Martinez Canillas
@ 2014-07-29 17:18   ` Mark Brown
  2014-07-30  8:49     ` Javier Martinez Canillas
  0 siblings, 1 reply; 16+ messages in thread
From: Mark Brown @ 2014-07-29 17:18 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Jul 29, 2014 at 06:28:57PM +0200, Javier Martinez Canillas wrote:
> If a selector can't be used on a platform due to voltage constraints,
> regulator_list_voltage() returns 0. Doing this unconditionally made
> sense since constraints were set in machine_constraints_voltage() at
> regulator registration time.
> 
> But for load switches that don't define a voltage output, the parent
> supply voltage is used so the constraints should only be applied if
> they were defined for the child regulators.

No, think about what you're doing here and why we're filtering out
unsettable voltages - this causes problems for consumers on regulators
that don't have any ability to vary voltages since they will now be able
to list voltages that they can't select.  

I would also expect any regulator where the supplied devices are able to
vary the voltage to explicitly provide a constraint even if the
implementation is done in a parent regulator.  There may be constraints
on the child supply which aren't directly present on the parent supply
and can be ignored if the child supply is turned off.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140729/015c4ddf/attachment.sig>

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

* [RFC 5/5] ARM: dts: Improve Peach Pit and Pi power scheme model
  2014-07-29 16:28 ` [RFC 5/5] ARM: dts: Improve Peach Pit and Pi power scheme model Javier Martinez Canillas
@ 2014-07-29 17:20   ` Mark Brown
  2014-07-30  8:53     ` Javier Martinez Canillas
  0 siblings, 1 reply; 16+ messages in thread
From: Mark Brown @ 2014-07-29 17:20 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Jul 29, 2014 at 06:28:59PM +0200, Javier Martinez Canillas wrote:
> The DeviceTree files for the Peach Pit and Pi machines have
> a simplistic model of the connections between the different
> regulators since not all the tps65090 regulators get their
> input supply voltage from the VDC. DCDC1-3, LD0-1 and fet7
> parent supply is indded VDC but the fet1-6 get their input
> supply from the DCDC1 and DCDC2 output voltage rails.

Acked-by: Mark Brown <broonie@linaro.org>

This *should* be independent of the rest of this series.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140729/374fee28/attachment-0001.sig>

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

* [RFC 4/5] regulator: tps65090: Set voltage for fixed regulators
  2014-07-29 16:28 ` [RFC 4/5] regulator: tps65090: Set voltage for fixed regulators Javier Martinez Canillas
@ 2014-07-29 17:36   ` Mark Brown
  2014-07-30  8:55     ` Javier Martinez Canillas
  0 siblings, 1 reply; 16+ messages in thread
From: Mark Brown @ 2014-07-29 17:36 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Jul 29, 2014 at 06:28:58PM +0200, Javier Martinez Canillas wrote:

> +#define tps65090_REG_VAR(_id, _sname, en_reg, _en_bits, _ops) \
> +	tps65090_REG_DESC(_id, _sname, en_reg, _en_bits, 0, 0, _ops)

I'd expect this to describe a variable regulator when in fact it
seems it describes a switch with no regulation.  Better name please.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140729/dad4d276/attachment-0001.sig>

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

* [RFC 2/5] regulator: core: Allow to get voltage count and list from parent
  2014-07-29 16:28 ` [RFC 2/5] regulator: core: Allow to get voltage count and list from parent Javier Martinez Canillas
@ 2014-07-29 18:25   ` Mark Brown
  0 siblings, 0 replies; 16+ messages in thread
From: Mark Brown @ 2014-07-29 18:25 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Jul 29, 2014 at 06:28:56PM +0200, Javier Martinez Canillas wrote:
> Load switches are modeled as regulators but they just provide
> the voltage of their parent input supply. So, the drivers for
> these switches usually neither provide a .list_voltage handler
> not set a .n_voltages count. But there is code in the kernel
> that assumes that all regulators should be able to provide this
> information (e.g: cpufreq and mmc subsystems).

Applied, thanks.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140729/4ee88451/attachment.sig>

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

* [RFC 1/5] regulator: core: Get voltage from parent if not available
  2014-07-29 16:28 ` [RFC 1/5] regulator: core: Get voltage from parent if not available Javier Martinez Canillas
@ 2014-07-29 18:26   ` Mark Brown
  2014-07-30  9:01     ` Javier Martinez Canillas
  0 siblings, 1 reply; 16+ messages in thread
From: Mark Brown @ 2014-07-29 18:26 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Jul 29, 2014 at 06:28:55PM +0200, Javier Martinez Canillas wrote:
> Load switches are modeled as regulators but they just provide
> the voltage of their parent input supply. So the drivers for

Applied, thanks.  The term "load switch" is a bit unusual - they're
usually just called switches (or sometimes FETs since that tends to be
the implementation).
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140729/310499cd/attachment.sig>

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

* [RFC 3/5] regulator: core: Only apply constraints if available on list voltage
  2014-07-29 17:18   ` Mark Brown
@ 2014-07-30  8:49     ` Javier Martinez Canillas
  2014-07-30 10:42       ` Mark Brown
  0 siblings, 1 reply; 16+ messages in thread
From: Javier Martinez Canillas @ 2014-07-30  8:49 UTC (permalink / raw)
  To: linux-arm-kernel

Hello Mark, thanks a lot for all your feedback.

On 07/29/2014 07:18 PM, Mark Brown wrote:
> On Tue, Jul 29, 2014 at 06:28:57PM +0200, Javier Martinez Canillas wrote:
>> 
>> But for load switches that don't define a voltage output, the parent
>> supply voltage is used so the constraints should only be applied if
>> they were defined for the child regulators.
> 
> No, think about what you're doing here and why we're filtering out
> unsettable voltages - this causes problems for consumers on regulators
> that don't have any ability to vary voltages since they will now be able
> to list voltages that they can't select.  
> 

Understood. Thanks for the explanation.

> I would also expect any regulator where the supplied devices are able to
> vary the voltage to explicitly provide a constraint even if the
> implementation is done in a parent regulator.  There may be constraints
> on the child supply which aren't directly present on the parent supply
> and can be ignored if the child supply is turned off.
> 

Agreed, if I explicitly set the voltage constraints on the child supply then
this patch is indeed not needed.

Best regards,
Javier

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

* [RFC 5/5] ARM: dts: Improve Peach Pit and Pi power scheme model
  2014-07-29 17:20   ` Mark Brown
@ 2014-07-30  8:53     ` Javier Martinez Canillas
  0 siblings, 0 replies; 16+ messages in thread
From: Javier Martinez Canillas @ 2014-07-30  8:53 UTC (permalink / raw)
  To: linux-arm-kernel

On 07/29/2014 07:20 PM, Mark Brown wrote:
> On Tue, Jul 29, 2014 at 06:28:59PM +0200, Javier Martinez Canillas wrote:
>> The DeviceTree files for the Peach Pit and Pi machines have
>> a simplistic model of the connections between the different
>> regulators since not all the tps65090 regulators get their
>> input supply voltage from the VDC. DCDC1-3, LD0-1 and fet7
>> parent supply is indded VDC but the fet1-6 get their input
>> supply from the DCDC1 and DCDC2 output voltage rails.
> 
> Acked-by: Mark Brown <broonie@linaro.org>
> 

Thanks.

> This *should* be independent of the rest of this series.
> 

Yes, it is independent as I explained in the cover letter. I just added in the
series since it was an RFC and I wanted you to have the complete picture.

I'll post it as an separate patch with your ack, sorry if this may cause an
inconvenience to you.

Best regards,
Javier

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

* [RFC 4/5] regulator: tps65090: Set voltage for fixed regulators
  2014-07-29 17:36   ` Mark Brown
@ 2014-07-30  8:55     ` Javier Martinez Canillas
  0 siblings, 0 replies; 16+ messages in thread
From: Javier Martinez Canillas @ 2014-07-30  8:55 UTC (permalink / raw)
  To: linux-arm-kernel

On 07/29/2014 07:36 PM, Mark Brown wrote:
> On Tue, Jul 29, 2014 at 06:28:58PM +0200, Javier Martinez Canillas wrote:
> 
>> +#define tps65090_REG_VAR(_id, _sname, en_reg, _en_bits, _ops) \
>> +	tps65090_REG_DESC(_id, _sname, en_reg, _en_bits, 0, 0, _ops)
> 
> I'd expect this to describe a variable regulator when in fact it
> seems it describes a switch with no regulation.  Better name please.
> 

Yes, when writing the patch I thought of a switch as a non-fixed regulator whose
output voltage is variable and depend on its input supply voltage. That's why I
used VAR but I agree that is a bad name and I'll change it.

Thanks a lot and best regards,
Javier

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

* [RFC 1/5] regulator: core: Get voltage from parent if not available
  2014-07-29 18:26   ` Mark Brown
@ 2014-07-30  9:01     ` Javier Martinez Canillas
  0 siblings, 0 replies; 16+ messages in thread
From: Javier Martinez Canillas @ 2014-07-30  9:01 UTC (permalink / raw)
  To: linux-arm-kernel

On 07/29/2014 08:26 PM, Mark Brown wrote:
> On Tue, Jul 29, 2014 at 06:28:55PM +0200, Javier Martinez Canillas wrote:
>> Load switches are modeled as regulators but they just provide
>> the voltage of their parent input supply. So the drivers for
> 
> Applied, thanks.  The term "load switch" is a bit unusual - they're
> usually just called switches (or sometimes FETs since that tends to be
> the implementation).
> 

Thanks, I used the terminology from the tps65090 data manual [0] that refers
these components as "load switches" all around the document. Although they are
called FETs indeed in the tables and register names.

Best regards,
Javier

[0]: http://www.ti.com/lit/gpn/tps65090

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

* [RFC 3/5] regulator: core: Only apply constraints if available on list voltage
  2014-07-30  8:49     ` Javier Martinez Canillas
@ 2014-07-30 10:42       ` Mark Brown
  0 siblings, 0 replies; 16+ messages in thread
From: Mark Brown @ 2014-07-30 10:42 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Jul 30, 2014 at 10:49:25AM +0200, Javier Martinez Canillas wrote:
> On 07/29/2014 07:18 PM, Mark Brown wrote:

> > I would also expect any regulator where the supplied devices are able to
> > vary the voltage to explicitly provide a constraint even if the
> > implementation is done in a parent regulator.  There may be constraints
> > on the child supply which aren't directly present on the parent supply
> > and can be ignored if the child supply is turned off.

> Agreed, if I explicitly set the voltage constraints on the child supply then
> this patch is indeed not needed.

We will, however, need some code to pass the child supply constraints on
to the parent.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140730/3d8206bc/attachment.sig>

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

end of thread, other threads:[~2014-07-30 10:42 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-29 16:28 [RFC 0/5] Get voltage from parent if not available on child Javier Martinez Canillas
2014-07-29 16:28 ` [RFC 1/5] regulator: core: Get voltage from parent if not available Javier Martinez Canillas
2014-07-29 18:26   ` Mark Brown
2014-07-30  9:01     ` Javier Martinez Canillas
2014-07-29 16:28 ` [RFC 2/5] regulator: core: Allow to get voltage count and list from parent Javier Martinez Canillas
2014-07-29 18:25   ` Mark Brown
2014-07-29 16:28 ` [RFC 3/5] regulator: core: Only apply constraints if available on list voltage Javier Martinez Canillas
2014-07-29 17:18   ` Mark Brown
2014-07-30  8:49     ` Javier Martinez Canillas
2014-07-30 10:42       ` Mark Brown
2014-07-29 16:28 ` [RFC 4/5] regulator: tps65090: Set voltage for fixed regulators Javier Martinez Canillas
2014-07-29 17:36   ` Mark Brown
2014-07-30  8:55     ` Javier Martinez Canillas
2014-07-29 16:28 ` [RFC 5/5] ARM: dts: Improve Peach Pit and Pi power scheme model Javier Martinez Canillas
2014-07-29 17:20   ` Mark Brown
2014-07-30  8:53     ` Javier Martinez Canillas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).