[PATCH 0/5] ARM: imx: ventana: enable LDO-bypass mode

[PATCH 0/5] ARM: imx: ventana: enable LDO-bypass mode

[Tim Harvey]

The IMX6 has some internal LDO regulators provided by the anatop regulator
block that can regulate the arm, soc, gpu/vpu core supplies. Alternatively a
design can supply vdd_arm and vdd_soc externally via a PMIC to provide a lower
power draw (switches are more efficient that ldo's).

The first two patches add some helper functions to the regulator core that
allow us to determine if two regulator are the same and if a regulator is
in bypass mode.

There are two patches which add device-tree support for specifying core
regulators.

The patch to the imx6q cpufreq driver is where the logic that determines if
we should bypass the LDO's and attempts to do so. I chose to place the logic
there as this the driver that is affecting changes on the core regulators.

This is a followup to an RFC I posted previously [1] and the response I
recieved from Lucas. I feel that this approach is an improvement over the
initial proposal.

Hopefully I have Cc'd the proper parties - please advise if not as I'm not
extactly clear who should be in the loop on regulator core changes.

Tim

[1] http://www.spinics.net/lists/arm-kernel/msg365686.html

Tim Harvey (5):
  regulator: add function to determine if 2 regulators are the same
  regulator: add function to determine if a regulator is in bypass mode
  ARM: dts: imx: add cpu0 alias
  cpufreq: imx6q: add ldo-bypass support
  ARM: imx: ventana: enable LDO bypass mode for GW54xx

 arch/arm/boot/dts/imx6dl.dtsi         |  2 +-
 arch/arm/boot/dts/imx6q.dtsi          |  2 +-
 arch/arm/boot/dts/imx6qdl-gw54xx.dtsi | 55 +++++++++++++++++++++++++----------
 drivers/cpufreq/imx6q-cpufreq.c       | 51 ++++++++++++++++++++++++++++++++
 drivers/regulator/core.c              | 40 +++++++++++++++++++++++++
 include/linux/regulator/consumer.h    | 14 +++++++++
 6 files changed, 146 insertions(+), 18 deletions(-)

-- 
1.8.3.2

[PATCH 1/5] regulator: add function to determine if 2 regulators are the same

[Tim Harvey]

Regulator structures can point to the same internal regulator dev's but as
that information is private we need to expose a function to determine if
two regulators do so.

Cc: linux-pm at vger.kernel.org
Signed-off-by: Tim Harvey <tharvey@gateworks.com>
---
 drivers/regulator/core.c           | 17 +++++++++++++++++
 include/linux/regulator/consumer.h |  8 ++++++++
 2 files changed, 25 insertions(+)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index cd87c0c..6d4a627 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2184,6 +2184,23 @@ int regulator_is_enabled(struct regulator *regulator)
 EXPORT_SYMBOL_GPL(regulator_is_enabled);
 
 /**
+ * regulator_is_same - are two regulators the same
+ * @regulator1: regulator source
+ * @regulator2: regulator source
+ *
+ * Returns positive if both regulators point to the same regulator_dev.
+ */
+int regulator_is_same(struct regulator *regulator1,
+		      struct regulator *regulator2)
+{
+	if (!regulator1 || !regulator2)
+		return -EINVAL;
+
+	return (regulator1->rdev == regulator2->rdev) ? 1 : 0;
+}
+EXPORT_SYMBOL_GPL(regulator_is_same);
+
+/**
  * regulator_can_change_voltage - check if regulator can change voltage
  * @regulator: regulator source
  *
diff --git a/include/linux/regulator/consumer.h b/include/linux/regulator/consumer.h
index d347c80..972090a 100644
--- a/include/linux/regulator/consumer.h
+++ b/include/linux/regulator/consumer.h
@@ -200,6 +200,8 @@ int __must_check regulator_enable(struct regulator *regulator);
 int regulator_disable(struct regulator *regulator);
 int regulator_force_disable(struct regulator *regulator);
 int regulator_is_enabled(struct regulator *regulator);
+int regulator_is_same(struct regulator *regulator1,
+		      struct regulator *regulator2);
 int regulator_disable_deferred(struct regulator *regulator, int ms);
 
 int __must_check regulator_bulk_get(struct device *dev, int num_consumers,
@@ -387,6 +389,12 @@ static inline int regulator_is_enabled(struct regulator *regulator)
 	return 1;
 }
 
+static inline int regulator_is_same(struct regulator *regulator1,
+				    struct regulator *regulator2)
+{
+	return 0;
+}
+
 static inline int regulator_bulk_get(struct device *dev,
 				     int num_consumers,
 				     struct regulator_bulk_data *consumers)
-- 
1.8.3.2

[PATCH 2/5] regulator: add function to determine if a regulator is in bypass mode

[Tim Harvey]

Regulators can be configured to allow bypass mode and can be told by a consumer 
that they are allowed to go into bypass mode from that consumer's perspective.
This adds a function to determine if the regulator actually has gone into
bypass mode (meaning all consumers allowed it to do so).

Cc: linux-pm at vger.kernel.org
Signed-off-by: Tim Harvey <tharvey@gateworks.com>
---
 drivers/regulator/core.c           | 23 +++++++++++++++++++++++
 include/linux/regulator/consumer.h |  6 ++++++
 2 files changed, 29 insertions(+)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 6d4a627..904c271 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -3150,6 +3150,29 @@ int regulator_allow_bypass(struct regulator *regulator, bool enable)
 EXPORT_SYMBOL_GPL(regulator_allow_bypass);
 
 /**
+ * regulator_is_bypass - determine if a regulator is in bypass mode
+ *
+ * @regulator: Regulator to configure
+ *
+ * return positive if regulator is in bypass mode.
+ */
+int regulator_is_bypass(struct regulator *regulator)
+{
+	struct regulator_dev *rdev = regulator->rdev;
+	int ret;
+
+	if (!regulator)
+		return -EINVAL;
+
+	mutex_lock(&rdev->mutex);
+	ret = regulator->bypass ? 1 : 0;
+	mutex_unlock(&rdev->mutex);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(regulator_is_bypass);
+
+/**
  * regulator_register_notifier - register regulator event notifier
  * @regulator: regulator source
  * @nb: notifier block
diff --git a/include/linux/regulator/consumer.h b/include/linux/regulator/consumer.h
index 972090a..a766946 100644
--- a/include/linux/regulator/consumer.h
+++ b/include/linux/regulator/consumer.h
@@ -237,6 +237,7 @@ unsigned int regulator_get_mode(struct regulator *regulator);
 int regulator_set_optimum_mode(struct regulator *regulator, int load_uA);
 
 int regulator_allow_bypass(struct regulator *regulator, bool allow);
+int regulator_is_bypass(struct regulator *regulator);
 
 struct regmap *regulator_get_regmap(struct regulator *regulator);
 int regulator_get_hardware_vsel_register(struct regulator *regulator,
@@ -493,6 +494,11 @@ static inline int regulator_allow_bypass(struct regulator *regulator,
 	return 0;
 }
 
+static inline int regulator_is_bypass(struct regulator *regulator)
+{
+	return 0;
+}
+
 static inline struct regmap *regulator_get_regmap(struct regulator *regulator)
 {
 	return ERR_PTR(-EOPNOTSUPP);
-- 
1.8.3.2

[PATCH 3/5] ARM: dts: imx: add cpu0 alias

[Tim Harvey]

Adding an alias on the cpu0 node allows devicetree files to modify the
regulators for LDO-bypass mode.

Cc: devicetree at vger.kernel.org
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Shawn Guo <shawn.guo@freescale.com>
Signed-off-by: Tim Harvey <tharvey@gateworks.com>
---
 arch/arm/boot/dts/imx6dl.dtsi | 2 +-
 arch/arm/boot/dts/imx6q.dtsi  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/imx6dl.dtsi b/arch/arm/boot/dts/imx6dl.dtsi
index b453e0e..61e0047 100644
--- a/arch/arm/boot/dts/imx6dl.dtsi
+++ b/arch/arm/boot/dts/imx6dl.dtsi
@@ -17,7 +17,7 @@
 		#address-cells = <1>;
 		#size-cells = <0>;
 
-		cpu at 0 {
+		cpu0: cpu at 0 {
 			compatible = "arm,cortex-a9";
 			device_type = "cpu";
 			reg = <0>;
diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
index e9f3646..5900761 100644
--- a/arch/arm/boot/dts/imx6q.dtsi
+++ b/arch/arm/boot/dts/imx6q.dtsi
@@ -21,7 +21,7 @@
 		#address-cells = <1>;
 		#size-cells = <0>;
 
-		cpu at 0 {
+		cpu0: cpu at 0 {
 			compatible = "arm,cortex-a9";
 			device_type = "cpu";
 			reg = <0>;
-- 
1.8.3.2

[PATCH 4/5] cpufreq: imx6q: add ldo-bypass support

[Tim Harvey]

When an external PMIC is used for VDD_SOC and VDD_ARM you can save power by
bypassing the internal LDO's provided by the anantop regulator as long as
you are running less than 1.2GHz. If running at 1.2GHz the IMX6 datasheets
state that you must use the internal LDO's to reduce ripple on the suplies.

A failure to bypass the LDO's when using an external PMIC will result in an
extra voltage drop (~125mV) between VDD_ARM_IN and VDD_ARM and VDD_SOC_IN and
VDD_SOC which violates the voltages specificed by the datasheets.

Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Shawn Guo <shawn.guo@freescale.com>
Signed-off-by: Tim Harvey <tharvey@gateworks.com>
---
 drivers/cpufreq/imx6q-cpufreq.c | 51 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/drivers/cpufreq/imx6q-cpufreq.c b/drivers/cpufreq/imx6q-cpufreq.c
index c2d3076..fc69f36 100644
--- a/drivers/cpufreq/imx6q-cpufreq.c
+++ b/drivers/cpufreq/imx6q-cpufreq.c
@@ -159,6 +159,9 @@ static int imx6q_cpufreq_probe(struct platform_device *pdev)
 	int num, ret;
 	const struct property *prop;
 	const __be32 *val;
+	struct regulator *anatop_arm_reg = NULL;
+	struct regulator *anatop_pu_reg = NULL;
+	struct regulator *anatop_soc_reg = NULL;
 	u32 nr, i, j;
 
 	cpu_dev = get_cpu_device(0);
@@ -167,6 +170,20 @@ static int imx6q_cpufreq_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
+	np = of_find_compatible_node(NULL, NULL, "fsl,imx6q-anatop");
+	if (np) {
+		anatop_arm_reg = regulator_get(&pdev->dev, "vddarm");
+		anatop_pu_reg = regulator_get(&pdev->dev, "vddpu");
+		anatop_soc_reg = regulator_get(&pdev->dev, "vddsoc");
+		if (PTR_ERR(anatop_arm_reg) == -EPROBE_DEFER ||
+		    PTR_ERR(anatop_pu_reg) == -EPROBE_DEFER ||
+		    PTR_ERR(anatop_soc_reg) == -EPROBE_DEFER) {
+				ret = -EPROBE_DEFER;
+				goto put_reg;
+		}
+		of_node_put(np);
+	}
+
 	np = of_node_get(cpu_dev->of_node);
 	if (!np) {
 		dev_err(cpu_dev, "failed to find cpu0 node\n");
@@ -301,7 +318,41 @@ soc_opp_out:
 		goto free_freq_table;
 	}
 
+	/* enable LDO bypass mode if anatop regs are not being used for core */
+	if ((!IS_ERR(anatop_arm_reg) &&
+	     !IS_ERR(anatop_pu_reg) &&
+	     !IS_ERR(anatop_soc_reg) &&
+	     regulator_is_same(arm_reg, anatop_arm_reg) &&
+	     regulator_is_same(pu_reg, anatop_pu_reg) &&
+	     regulator_is_same(soc_reg, anatop_soc_reg)) ||
+            (freq_table[num].frequency == FREQ_1P2_GHZ / 1000))
+	{
+		printk("Using anatop regulators: LDO enabled\n");
+	} else {
+		int puvolt = regulator_get_voltage(anatop_pu_reg);
+
+		printk("Not using anatop LDO's: enabling LDO bypass\n");
+		regulator_allow_bypass(anatop_arm_reg, true);
+		regulator_allow_bypass(anatop_pu_reg, true);
+		if (regulator_is_same(pu_reg, anatop_pu_reg))
+			regulator_allow_bypass(pu_reg, true);
+		regulator_allow_bypass(anatop_soc_reg, true);
+		if (!regulator_is_bypass(anatop_arm_reg) ||
+		    !regulator_is_bypass(anatop_pu_reg) ||
+		    !regulator_is_bypass(anatop_soc_reg))
+			dev_err(cpu_dev, "failed to set LDO bypass\n");
+		else {
+			if (puvolt == 0)
+				regulator_set_voltage(anatop_pu_reg, 0, 0);
+		}
+
+	}
+	regulator_put(anatop_arm_reg);
+	regulator_put(anatop_pu_reg);
+	regulator_put(anatop_soc_reg);
+
 	of_node_put(np);
+
 	return 0;
 
 free_freq_table:
-- 
1.8.3.2

[PATCH 5/5] ARM: imx: ventana: enable LDO bypass mode for GW54xx

[Tim Harvey]

The GW54xx baseboard has a PFUZE100 PMIC capable of regulating the
core voltages (VDD_ARM, VDD_SOC) externally such that the internal IMX6
anatop LDO regulators are not needed. This provides a power reduction
(as the PMIC is more efficient than the LDO's) as well as moves some
of the power/thermal burden from the IMX to the PMIC.

Cc: devicetree at vger.kernel.org
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Shawn Guo <shawn.guo@freescale.com>
Signed-off-by: Tim Harvey <tharvey@gateworks.com>
---
 arch/arm/boot/dts/imx6qdl-gw54xx.dtsi | 55 +++++++++++++++++++++++++----------
 1 file changed, 39 insertions(+), 16 deletions(-)

diff --git a/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi b/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi
index cf13239..3ab0872 100644
--- a/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi
@@ -142,6 +142,11 @@
 	status = "okay";
 };
 
+&cpu0 {
+	arm-supply = <&reg_vdd_arm>;
+	soc-supply = <&reg_vdd_soc>;
+};
+
 &fec {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_enet>;
@@ -215,7 +220,8 @@
 		reg = <0x08>;
 
 		regulators {
-			sw1a_reg: sw1ab {
+			/* VDD_ARM */
+			reg_vdd_arm: sw1ab {
 				regulator-min-microvolt = <300000>;
 				regulator-max-microvolt = <1875000>;
 				regulator-boot-on;
@@ -223,7 +229,8 @@
 				regulator-ramp-delay = <6250>;
 			};
 
-			sw1c_reg: sw1c {
+			/* VDD_SOC */
+			reg_vdd_soc: sw1c {
 				regulator-min-microvolt = <300000>;
 				regulator-max-microvolt = <1875000>;
 				regulator-boot-on;
@@ -231,77 +238,93 @@
 				regulator-ramp-delay = <6250>;
 			};
 
-			sw2_reg: sw2 {
+			/* VDD_HIGH */
+			reg_3p0v: sw2 {
 				regulator-min-microvolt = <800000>;
 				regulator-max-microvolt = <3950000>;
 				regulator-boot-on;
 				regulator-always-on;
 			};
 
-			sw3a_reg: sw3a {
+			/* VDD_DDR */
+			reg_ddra: sw3a {
 				regulator-min-microvolt = <400000>;
 				regulator-max-microvolt = <1975000>;
 				regulator-boot-on;
 				regulator-always-on;
 			};
 
-			sw3b_reg: sw3b {
+			/* VDD_DDR */
+			reg_ddrb: sw3b {
 				regulator-min-microvolt = <400000>;
 				regulator-max-microvolt = <1975000>;
 				regulator-boot-on;
 				regulator-always-on;
 			};
 
-			sw4_reg: sw4 {
+			/* VDD_1P8 */
+			reg_1p8v: sw4 {
 				regulator-min-microvolt = <800000>;
 				regulator-max-microvolt = <3300000>;
 			};
 
-			swbst_reg: swbst {
+			/* VDD_5P0 */
+			reg_swbst: swbst {
 				regulator-min-microvolt = <5000000>;
 				regulator-max-microvolt = <5150000>;
 			};
 
-			snvs_reg: vsnvs {
+			reg_snvs: vsnvs {
 				regulator-min-microvolt = <1000000>;
 				regulator-max-microvolt = <3000000>;
 				regulator-boot-on;
 				regulator-always-on;
 			};
 
-			vref_reg: vrefddr {
+			/* VDD_VREF */
+			reg_vref: vrefddr {
 				regulator-boot-on;
 				regulator-always-on;
 			};
 
-			vgen1_reg: vgen1 {
+			/* VDD_PCI_1P5_1 */
+			reg_1p5va: vgen1 {
 				regulator-min-microvolt = <800000>;
 				regulator-max-microvolt = <1550000>;
+				regulator-boot-on;
+				regulator-always-on;
 			};
 
-			vgen2_reg: vgen2 {
+			/* VDD_PCI_1P5_2 */
+			reg_1p5vb: vgen2 {
 				regulator-min-microvolt = <800000>;
 				regulator-max-microvolt = <1550000>;
+				regulator-boot-on;
+				regulator-always-on;
 			};
 
-			vgen3_reg: vgen3 {
+			/* unused */
+			reg_vgen3: vgen3 {
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <3300000>;
 			};
 
-			vgen4_reg: vgen4 {
+			/* VDD_AUD_1P8 */
+			reg_aud_1p8v: vgen4 {
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <3300000>;
 				regulator-always-on;
 			};
 
-			vgen5_reg: vgen5 {
+			/* VDD_2P5 */
+			reg_2p5v: vgen5 {
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <3300000>;
 				regulator-always-on;
 			};
 
-			vgen6_reg: vgen6 {
+			/* unused */
+			reg_vgen6: vgen6 {
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <3300000>;
 				regulator-always-on;
@@ -320,7 +343,7 @@
 		compatible = "fsl,sgtl5000";
 		reg = <0x0a>;
 		clocks = <&clks 201>;
-		VDDA-supply = <&sw4_reg>;
+		VDDA-supply = <&reg_1p8v>;
 		VDDIO-supply = <&reg_3p3v>;
 	};
 
-- 
1.8.3.2

[PATCH 4/5] cpufreq: imx6q: add ldo-bypass support

[Fabio Estevam]

Hi Tim,

On Fri, Oct 31, 2014 at 2:27 AM, Tim Harvey <tharvey@gateworks.com> wrote:
> When an external PMIC is used for VDD_SOC and VDD_ARM you can save power by
> bypassing the internal LDO's provided by the anantop regulator as long as
> you are running less than 1.2GHz. If running at 1.2GHz the IMX6 datasheets
> state that you must use the internal LDO's to reduce ripple on the suplies.
>
> A failure to bypass the LDO's when using an external PMIC will result in an
> extra voltage drop (~125mV) between VDD_ARM_IN and VDD_ARM and VDD_SOC_IN and
> VDD_SOC which violates the voltages specificed by the datasheets.
>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: Shawn Guo <shawn.guo@freescale.com>
> Signed-off-by: Tim Harvey <tharvey@gateworks.com>
> ---
>  drivers/cpufreq/imx6q-cpufreq.c | 51 +++++++++++++++++++++++++++++++++++++++++

Shouldn't the LDO bypass support be added into the anatop regulator
driver instead?

What if someone wants to use LDO bypass and not use the cpufreq driver?

[PATCH 4/5] cpufreq: imx6q: add ldo-bypass support

[Tim Harvey]

On Wed, Dec 17, 2014 at 6:36 AM, Fabio Estevam <festevam@gmail.com> wrote:
> Hi Tim,
>
> On Fri, Oct 31, 2014 at 2:27 AM, Tim Harvey <tharvey@gateworks.com> wrote:
>> When an external PMIC is used for VDD_SOC and VDD_ARM you can save power by
>> bypassing the internal LDO's provided by the anantop regulator as long as
>> you are running less than 1.2GHz. If running at 1.2GHz the IMX6 datasheets
>> state that you must use the internal LDO's to reduce ripple on the suplies.
>>
>> A failure to bypass the LDO's when using an external PMIC will result in an
>> extra voltage drop (~125mV) between VDD_ARM_IN and VDD_ARM and VDD_SOC_IN and
>> VDD_SOC which violates the voltages specificed by the datasheets.
>>
>> Cc: Philipp Zabel <p.zabel@pengutronix.de>
>> Cc: Shawn Guo <shawn.guo@freescale.com>
>> Signed-off-by: Tim Harvey <tharvey@gateworks.com>
>> ---
>>  drivers/cpufreq/imx6q-cpufreq.c | 51 +++++++++++++++++++++++++++++++++++++++++
>
> Shouldn't the LDO bypass support be added into the anatop regulator
> driver instead?
>
> What if someone wants to use LDO bypass and not use the cpufreq driver?

Fabio,

That does make sense perhaps - I'm just not sure the best way to do it.

What is needed is to determine if the cpu vddsoc and vddarm regulators
are both 'not' the same as the anatop provided regulators (then we
bypass the anatop regulators) so I need to do such a check after all
regulators are registered. Perhaps I need to have a late_init call (or
some other init call that happens after all regulators are
registered).

Phillipp/Mark - what are your thoughts here? Do the regulator core
functions regulator_is_same() [1] and regulator_is_bypass() [2] I
propose make sense to determine if regulators are the same and in
bypass mode and overcome the detection issues Phillipp discussed in a
previous thread [3]?

[1] http://article.gmane.org/gmane.linux.kernel/1818320/
[2] http://article.gmane.org/gmane.linux.kernel/1818324/
[3] http://thread.gmane.org/gmane.linux.ports.arm.kernel/359856

Regards,

Tim

[PATCH 4/5] cpufreq: imx6q: add ldo-bypass support

[Mark Brown]

On Thu, Dec 18, 2014 at 06:11:15AM -0800, Tim Harvey wrote:
> On Wed, Dec 17, 2014 at 6:36 AM, Fabio Estevam <festevam@gmail.com> wrote:
> > On Fri, Oct 31, 2014 at 2:27 AM, Tim Harvey <tharvey@gateworks.com> wrote:
> >> When an external PMIC is used for VDD_SOC and VDD_ARM you can save power by
> >> bypassing the internal LDO's provided by the anantop regulator as long as
> >> you are running less than 1.2GHz. If running at 1.2GHz the IMX6 datasheets
> >> state that you must use the internal LDO's to reduce ripple on the suplies.
> >>
> >> A failure to bypass the LDO's when using an external PMIC will result in an
> >> extra voltage drop (~125mV) between VDD_ARM_IN and VDD_ARM and VDD_SOC_IN and
> >> VDD_SOC which violates the voltages specificed by the datasheets.

This description doesn't make much sense - there must of course always
be an external power source for the SoC and the discussion of bypassing
also suggests that it's not just a case of disconnecting the internal
LDOs.

> What is needed is to determine if the cpu vddsoc and vddarm regulators
> are both 'not' the same as the anatop provided regulators (then we
> bypass the anatop regulators) so I need to do such a check after all
> regulators are registered. Perhaps I need to have a late_init call (or
> some other init call that happens after all regulators are
> registered).

> Phillipp/Mark - what are your thoughts here? Do the regulator core
> functions regulator_is_same() [1] and regulator_is_bypass() [2] I
> propose make sense to determine if regulators are the same and in
> bypass mode and overcome the detection issues Phillipp discussed in a
> previous thread [3]?

Please provide a clear description of what's actually going on here.
What does the hardware actually look like and what is being configured?
You're telling me the solution you've decided on, not what the problem
that this is supposed to solve is.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141218/2741c50e/attachment.sig>

[PATCH 4/5] cpufreq: imx6q: add ldo-bypass support

[Tim Harvey]

On Thu, Dec 18, 2014 at 6:22 AM, Mark Brown <broonie@kernel.org> wrote:
> On Thu, Dec 18, 2014 at 06:11:15AM -0800, Tim Harvey wrote:
>> On Wed, Dec 17, 2014 at 6:36 AM, Fabio Estevam <festevam@gmail.com> wrote:
>> > On Fri, Oct 31, 2014 at 2:27 AM, Tim Harvey <tharvey@gateworks.com> wrote:
>> >> When an external PMIC is used for VDD_SOC and VDD_ARM you can save power by
>> >> bypassing the internal LDO's provided by the anantop regulator as long as
>> >> you are running less than 1.2GHz. If running at 1.2GHz the IMX6 datasheets
>> >> state that you must use the internal LDO's to reduce ripple on the suplies.
>> >>
>> >> A failure to bypass the LDO's when using an external PMIC will result in an
>> >> extra voltage drop (~125mV) between VDD_ARM_IN and VDD_ARM and VDD_SOC_IN and
>> >> VDD_SOC which violates the voltages specificed by the datasheets.
>
> This description doesn't make much sense - there must of course always
> be an external power source for the SoC and the discussion of bypassing
> also suggests that it's not just a case of disconnecting the internal
> LDOs.
>
>> What is needed is to determine if the cpu vddsoc and vddarm regulators
>> are both 'not' the same as the anatop provided regulators (then we
>> bypass the anatop regulators) so I need to do such a check after all
>> regulators are registered. Perhaps I need to have a late_init call (or
>> some other init call that happens after all regulators are
>> registered).
>
>> Phillipp/Mark - what are your thoughts here? Do the regulator core
>> functions regulator_is_same() [1] and regulator_is_bypass() [2] I
>> propose make sense to determine if regulators are the same and in
>> bypass mode and overcome the detection issues Phillipp discussed in a
>> previous thread [3]?
>
> Please provide a clear description of what's actually going on here.
> What does the hardware actually look like and what is being configured?
> You're telling me the solution you've decided on, not what the problem
> that this is supposed to solve is.

Mark,

I can embellish the description with more information. Does this
describe the issue better?

---

The IMX6 CPU family (IMX6S/IMX6DL/IMX6D/IMX6Q) have three CPU related
voltage rails: VDD_ARM (for the ARM core(s)), VDD_SOC (for the SoC
logic), and VDD_PU (for the GPU and VPU). All three of these have
voltage setpoints recommended by the datasheet which vary per CPU
frequency. They also all have internal LDO's (implemented via the
anantop voltage regulator) such that a board does not have to have a
complex PMIC with scalable voltages. The VDD_PU LDO is internally tied
to the VDD_SOC input. Therefore, there are two external power inputs
to the chip: VDD_ARM and VDD_SOC. When using a non scalable PMIC you
can provide VDD_ARM and VDD_SOC with a set voltage level specified by
the datasheet, and use the three internal LDO's inside the IMX6 to
adjust the three power rails down to the proper value per CPU
frequency setpoint. If however you have an external PMIC that is
capable of adjusting the VDD_ARM and VDD_SOC to the IMX6 and an
appropriate regulator driver is capable of doing so, you must
configure the cpu nodes vddarm and vddsoc regulators to those provided
by your PMIC and 'bypass' the internal LDO's by setting their scaling
value to full-scale (0x1f). Failure to place the internal LDO's in
'bypass mode' will result in a minimum of 125mV drop from the external
input to the internal voltage rail such that the CPU operating points
describing the voltage of the external inputs per CPU frequency will
not be properly met.

An additional benefit of using a PMIC with a regulator driver is that
you can save power regulating the voltage with a switching regulator
PMIC vs an inefficient LDO within the CPU. This also will help to move
thermal power out of the IMX6 and to your PMIC.

If running at 1.2GHz the IMX6 datasheets state that you must use the
internal LDO's to reduce ripple on the supplies.

----

Maybe Fabio has something to add? Perhaps this should go somewhere
under Documentation but I'm not clear where.

Tim

[PATCH 4/5] cpufreq: imx6q: add ldo-bypass support

[Mark Brown]

On Fri, Dec 19, 2014 at 08:17:41AM -0800, Tim Harvey wrote:

> frequency setpoint. If however you have an external PMIC that is
> capable of adjusting the VDD_ARM and VDD_SOC to the IMX6 and an
> appropriate regulator driver is capable of doing so, you must
> configure the cpu nodes vddarm and vddsoc regulators to those provided
> by your PMIC and 'bypass' the internal LDO's by setting their scaling

You're describing the implementation again, not the system.  Describe
the system then if you want to describe the solution you've come up with
for modelling it do so separately.

Please also try to use paragraphs to break up what you're saying a bit.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141219/e93f6f4b/attachment-0001.sig>

[PATCH 4/5] cpufreq: imx6q: add ldo-bypass support

[Tim Harvey]

On Fri, Dec 19, 2014 at 8:23 AM, Mark Brown <broonie@kernel.org> wrote:
> On Fri, Dec 19, 2014 at 08:17:41AM -0800, Tim Harvey wrote:
>
>> frequency setpoint. If however you have an external PMIC that is
>> capable of adjusting the VDD_ARM and VDD_SOC to the IMX6 and an
>> appropriate regulator driver is capable of doing so, you must
>> configure the cpu nodes vddarm and vddsoc regulators to those provided
>> by your PMIC and 'bypass' the internal LDO's by setting their scaling
>
> You're describing the implementation again, not the system.  Describe
> the system then if you want to describe the solution you've come up with
> for modelling it do so separately.
>
> Please also try to use paragraphs to break up what you're saying a bit.

Mark,

I'm not sure I'm understanding what your asking for or how to describe
it differently. Fabio / Shawn / Philipp could you help me out here
with the proper explanation of the IMX6 LDO's so that we can make some
progress on getting LDO bypass implemented?

Regards,

Tim

[PATCH 4/5] cpufreq: imx6q: add ldo-bypass support

[Mark Brown]

On Mon, Jan 05, 2015 at 06:38:50AM -0800, Tim Harvey wrote:
> On Fri, Dec 19, 2014 at 8:23 AM, Mark Brown <broonie@kernel.org> wrote:
> > On Fri, Dec 19, 2014 at 08:17:41AM -0800, Tim Harvey wrote:

> >> frequency setpoint. If however you have an external PMIC that is
> >> capable of adjusting the VDD_ARM and VDD_SOC to the IMX6 and an
> >> appropriate regulator driver is capable of doing so, you must
> >> configure the cpu nodes vddarm and vddsoc regulators to those provided
> >> by your PMIC and 'bypass' the internal LDO's by setting their scaling

> > You're describing the implementation again, not the system.  Describe
> > the system then if you want to describe the solution you've come up with
> > for modelling it do so separately.

> > Please also try to use paragraphs to break up what you're saying a bit.

> I'm not sure I'm understanding what your asking for or how to describe
> it differently. Fabio / Shawn / Philipp could you help me out here
> with the proper explanation of the IMX6 LDO's so that we can make some
> progress on getting LDO bypass implemented?

I have no idea what the context is any more, sorry.  The above looks
like you're talking about the specifics of how you've implemented
something instead of describing the hardware you're trying to abstract -
you're taling about the "cpu nodes" in what sounds like it's supposed to
be a description of the hardware for example.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150105/ed045e38/attachment.sig>

[PATCH 0/5] ARM: imx: ventana: enable LDO-bypass mode

[Jean-Michel Hautbois]

Hi Tim,

2014-10-31 5:27 GMT+01:00 Tim Harvey <tharvey@gateworks.com>:
> The IMX6 has some internal LDO regulators provided by the anatop regulator
> block that can regulate the arm, soc, gpu/vpu core supplies. Alternatively a
> design can supply vdd_arm and vdd_soc externally via a PMIC to provide a lower
> power draw (switches are more efficient that ldo's).

What is the status of these patches ? My design uses a PFUZE100
regulator, and having this would be interesting.

Thanks,
JM

[PATCH 0/5] ARM: imx: ventana: enable LDO-bypass mode

[Jean-Michel Hautbois]

Hi Tim,

2015-03-24 16:11 GMT+01:00 Tim Harvey <tharvey@gateworks.com>:
> On Tue, Feb 24, 2015 at 1:36 PM, Jean-Michel Hautbois
> <jhautbois@gmail.com> wrote:
>> Hi Tim,
>>
>> 2014-10-31 5:27 GMT+01:00 Tim Harvey <tharvey@gateworks.com>:
>>> The IMX6 has some internal LDO regulators provided by the anatop regulator
>>> block that can regulate the arm, soc, gpu/vpu core supplies. Alternatively a
>>> design can supply vdd_arm and vdd_soc externally via a PMIC to provide a lower
>>> power draw (switches are more efficient that ldo's).
>>
>> What is the status of these patches ? My design uses a PFUZE100
>> regulator, and having this would be interesting.
>>
>> Thanks,
>> JM
>
> JM,
>
> No update from me - I haven't had time to get back to this
> unfortunately. Most of our customers are using the Freescale vendor
> kernel and I have ldo-bypass working fine there, just not in a way
> that is easily mainlineable without work.


OK, thank you...
It's ashame that Freescale won't do a proper job to mainline
everything, it would be good for them too !
Well, If I get some time to look into this I will probably start from
your proposal.

Thanks,
JM