[RFC PATCH v2 0/6] ARM: OMAP3+: Introduce ABB driver

[RFC PATCH v2 0/6] ARM: OMAP3+: Introduce ABB driver

[Andrii Tseglytskyi]

From: "Andrii.Tseglytskyi" <andrii.tseglytskyi@ti.com>

Following patch series introduces the Adaptive Body-Bias
LDO driver, which handles LDOs voltage during OPP change routine.
Current implementation is based on patch series from
Mike Turquette:

http://marc.info/?l=linux-omap&m=134931341818379&w=2

ABB transition is a part of OPP changing sequence.
ABB can operate in the following modes:
- Bypass mode: Activated when ABB is not required
- FBB mode: Fast Body Bias mode, used on fast OPPs
- RBB mode: Reverse Body Bias mode, used on slow OPPs

In current implementation ABB is converted to regulator.
Standalone OPP table is used to store ABB mode, it is defined
in device tree for each ABB regulator. It has the following format:

operating-points = <
	       /* uV   ABB (0 - Bypass, 1 - FBB, 2 - RBB) */
	       880000		0
	       1060000		1
	       1250000		1
	       1260000		1
>;

ABB regulator is linked to regulator chain.

Related discussions:
regulator: query on regulator re-entrance
http://marc.info/?l=linux-omap&m=136513861315970&w=2

regulator: core: introduce regulator chain locking scheme
https://patchwork.kernel.org/patch/2445091/

clk: notifier handler for dynamic voltage scaling
https://lkml.org/lkml/2013/2/27/414

Andrii.Tseglytskyi (6):
  ARM: dts: OMAP36xx: add device tree for ABB
  ARM: dts: OMAP4: add device tree for ABB
  ARM: dts: OMAP5: add device tree for ABB
  ARM: OMAP3+: ABB: add aliases for sysclk used in ABB driver
  ARM: OMAP3+: ABB: introduce ABB driver
  ARM: OMAP3+: ABB: introduce debugfs entry

 Documentation/devicetree/bindings/power/abb.txt |   38 ++
 arch/arm/boot/dts/omap36xx.dtsi                 |   20 +
 arch/arm/boot/dts/omap4.dtsi                    |   22 +
 arch/arm/boot/dts/omap443x.dtsi                 |   23 +
 arch/arm/boot/dts/omap446x.dtsi                 |   39 ++
 arch/arm/boot/dts/omap5.dtsi                    |   39 ++
 arch/arm/mach-omap2/cclock3xxx_data.c           |    1 +
 arch/arm/mach-omap2/cclock44xx_data.c           |    2 +
 drivers/regulator/Kconfig                       |    6 +
 drivers/regulator/Makefile                      |    1 +
 drivers/regulator/abb-regulator.c               |  710 +++++++++++++++++++++++
 11 files changed, 901 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/abb.txt
 create mode 100644 arch/arm/boot/dts/omap446x.dtsi
 create mode 100644 drivers/regulator/abb-regulator.c

-- 
1.7.9.5

[RFC PATCH v2 1/6] ARM: dts: OMAP36xx: add device tree for ABB

[Andrii Tseglytskyi]

From: "Andrii.Tseglytskyi" <andrii.tseglytskyi@ti.com>

Add DT ABB data for OMAP36xx family of devices.
Data is based on OMAP36XX TRM document.

Signed-off-by: Andrii.Tseglytskyi <andrii.tseglytskyi@ti.com>
---
 Documentation/devicetree/bindings/power/abb.txt |   38 +++++++++++++++++++++++
 arch/arm/boot/dts/omap36xx.dtsi                 |   20 ++++++++++++
 2 files changed, 58 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/abb.txt

diff --git a/Documentation/devicetree/bindings/power/abb.txt b/Documentation/devicetree/bindings/power/abb.txt
new file mode 100644
index 0000000..3f6c17c
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/abb.txt
@@ -0,0 +1,38 @@
+ABB regulator for OMAP platforms
+
+Required properties :
+- compatible:
+  - "ti,omap36xx-abb" for OMAP36XX
+  - "ti,omap4-abb" for OMAP4
+  - "ti,omap5-abb" for OMAP5
+
+- regulator-min-microvolt: lowest OPP voltage
+- regulator-max-microvolt: highest OPP voltage
+- regulator-always-on: ABB should be always on
+- reg: addres range defined in TRM
+- ti,tranxdone_status_mask: ABB transition state mask
+- operating-points: An array of 2-tuples items, and each item consists
+  of voltage and ABB opp_sel, like <volt-uV opp_sel>.
+	volt: voltage in uV
+	opp_sel: 0-bypass, 1-FBB, 2-RBB
+
+Examples :
+abb_mpu_iva: regulator-abb1 {
+	compatible = "ti,omap36xx-abb";
+	regulator-name = "abb_mpu_iva";
+	regulator-min-microvolt = <1012500>;
+	regulator-max-microvolt = <1375000>;
+	regulator-always-on;
+	#address-cells = <1>;
+	#size-cells = <1>;
+	reg = <0x483072f0 0x8>,
+	      <0x48306818 0x4>;
+	ti,tranxdone_status_mask = <0x4000000>;
+	operating-points = <
+		/* uV   ABB */
+		1012500 0
+		1200000 0
+		1325000 0
+		1375000 1
+	>;
+};
diff --git a/arch/arm/boot/dts/omap36xx.dtsi b/arch/arm/boot/dts/omap36xx.dtsi
index b89233e..5e917ca 100644
--- a/arch/arm/boot/dts/omap36xx.dtsi
+++ b/arch/arm/boot/dts/omap36xx.dtsi
@@ -28,6 +28,26 @@
 		};
 	};
 
+	abb_mpu_iva: regulator-abb1 {
+		compatible = "ti,omap36xx-abb";
+		regulator-name = "abb_mpu_iva";
+		regulator-min-microvolt = <1012500>;
+		regulator-max-microvolt = <1375000>;
+		regulator-always-on;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		reg = <0x483072f0 0x8>,
+		      <0x48306818 0x4>;
+		ti,tranxdone_status_mask = <0x4000000>;
+		operating-points = <
+			/* uV   ABB */
+			1012500	0
+			1200000	0
+			1325000	0
+			1375000	1
+		>;
+	};
+
 	ocp {
 		uart4: serial@49042000 {
 			compatible = "ti,omap3-uart";
-- 
1.7.9.5

[RFC PATCH v2 2/6] ARM: dts: OMAP4: add device tree for ABB

[Andrii Tseglytskyi]

From: "Andrii.Tseglytskyi" <andrii.tseglytskyi@ti.com>

Add DT ABB data for OMAP44xx family of devices.
Data is based on OMAP44xx TRM document.

Signed-off-by: Andrii.Tseglytskyi <andrii.tseglytskyi@ti.com>
---
 arch/arm/boot/dts/omap4.dtsi    |   22 ++++++++++++++++++++++
 arch/arm/boot/dts/omap443x.dtsi |   23 +++++++++++++++++++++++
 arch/arm/boot/dts/omap446x.dtsi |   39 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 84 insertions(+)
 create mode 100644 arch/arm/boot/dts/omap446x.dtsi

diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
index 2a56428..8a98002 100644
--- a/arch/arm/boot/dts/omap4.dtsi
+++ b/arch/arm/boot/dts/omap4.dtsi
@@ -81,6 +81,28 @@
 		};
 	};
 
+	abb_mpu: regulator-abb1 {
+		compatible = "ti,omap4-abb";
+		regulator-name = "abb_mpu";
+		regulator-always-on;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		reg = <0x4a307bd0 0x8>,
+		      <0x4a306014 0x4>;
+		ti,tranxdone_status_mask = <0x80>;
+	};
+
+	abb_iva: regulator-abb2 {
+		compatible = "ti,omap4-abb";
+		regulator-name = "abb_iva";
+		regulator-always-on;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		reg = <0x4a307bd8 0x8>,
+		      <0x4a306010 0x4>;
+		ti,tranxdone_status_mask = <0x80000000>;
+	};
+
 	/*
 	 * XXX: Use a flat representation of the OMAP4 interconnect.
 	 * The real OMAP interconnect network is quite complex.
diff --git a/arch/arm/boot/dts/omap443x.dtsi b/arch/arm/boot/dts/omap443x.dtsi
index cccf39a..044b54c 100644
--- a/arch/arm/boot/dts/omap443x.dtsi
+++ b/arch/arm/boot/dts/omap443x.dtsi
@@ -24,4 +24,27 @@
 			clock-latency = <300000>; /* From legacy driver */
 		};
 	};
+
+	abb_mpu: regulator-abb1 {
+		regulator-min-microvolt = <1025000>;
+		regulator-max-microvolt = <1375000>;
+		operating-points = <
+			/* uV   ABB */
+			1025000	0
+			1200000	0
+			1313000	0
+			1375000	1
+		>;
+	};
+
+	abb_iva: regulator-abb2 {
+		regulator-min-microvolt = <950000>;
+		regulator-max-microvolt = <1291000>;
+		operating-points = <
+			/* uV   ABB */
+			950000	0
+			1114000	0
+			1291000	0
+		>;
+	};
 };
diff --git a/arch/arm/boot/dts/omap446x.dtsi b/arch/arm/boot/dts/omap446x.dtsi
new file mode 100644
index 0000000..3c81d3b
--- /dev/null
+++ b/arch/arm/boot/dts/omap446x.dtsi
@@ -0,0 +1,39 @@
+/*
+ * Device Tree Source for OMAP446x SoC
+ *
+ * Copyright (C) 2013 Texas Instruments Incorporated - http://www.ti.com/
+ *
+ * This file is licensed under the terms of the GNU General Public License
+ * version 2.  This program is licensed "as is" without any warranty of any
+ * kind, whether express or implied.
+ */
+
+/include/ "omap4.dtsi"
+
+/ {
+	abb_mpu: regulator-abb1 {
+		regulator-min-microvolt = <1025000>;
+		regulator-max-microvolt = <1390000>;
+		operating-points = <
+			/* uV   ABB */
+			1025000	0
+			1203000	0
+			1317000	0
+			1380000	1
+			1390000	1
+		>;
+	};
+
+	abb_iva: regulator-abb2 {
+		regulator-min-microvolt = <950000>;
+		regulator-max-microvolt = <1376000>;
+		operating-points = <
+			/* uV   ABB */
+			950000	0
+			1140000	0
+			1291000	0
+			1375000	1
+			1376000	1
+		>;
+	};
+};
-- 
1.7.9.5

[RFC PATCH v2 3/6] ARM: dts: OMAP5: add device tree for ABB

[Andrii Tseglytskyi]

From: "Andrii.Tseglytskyi" <andrii.tseglytskyi@ti.com>

Add DT ABB data for OMAP543x family of devices.
Data is based on OMAP543x TRM document.

Signed-off-by: Andrii.Tseglytskyi <andrii.tseglytskyi@ti.com>
---
 arch/arm/boot/dts/omap5.dtsi |   39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
index 3dd7ff8..e1b2c92 100644
--- a/arch/arm/boot/dts/omap5.dtsi
+++ b/arch/arm/boot/dts/omap5.dtsi
@@ -74,6 +74,45 @@
 		};
 	};
 
+	abb_mpu: regulator-abb1 {
+		compatible = "ti,omap5-abb";
+		regulator-name = "abb_mpu";
+		regulator-min-microvolt = <880000>;
+		regulator-max-microvolt = <1260000>;
+		regulator-always-on;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		reg = <0x4ae07cdc 0x8>,
+		      <0x4ae06014 0x4>;
+		ti,tranxdone_status_mask = <0x80>;
+		operating-points = <
+			/* uV   ABB */
+			880000	0
+			1060000	1
+			1250000	1
+			1260000	1
+		>;
+	};
+
+	abb_iva: regulator-abb2 {
+		compatible = "ti,omap5-abb";
+		regulator-name = "abb_iva";
+		regulator-min-microvolt = <880000>;
+		regulator-max-microvolt = <1120000>;
+		regulator-always-on;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		reg = <0x4ae07ce4 0x8>,
+		      <0x4ae06010 0x4>;
+		ti,tranxdone_status_mask = <0x80000000>;
+		operating-points = <
+			/* uV   ABB */
+			880000	0
+			1025000	0
+			1120000	0
+		>;
+	};
+
 	/*
 	 * XXX: Use a flat representation of the OMAP3 interconnect.
 	 * The real OMAP interconnect network is quite complex.
-- 
1.7.9.5

[RFC PATCH v2 4/6] ARM: OMAP3+: ABB: add aliases for sysclk used in ABB driver

[Andrii Tseglytskyi]

From: "Andrii.Tseglytskyi" <andrii.tseglytskyi@ti.com>

This patch introduces aliases for SYS clock nedded for ABB
driver, which will be introduced in the following patch.

Signed-off-by: Andrii.Tseglytskyi <andrii.tseglytskyi@ti.com>
---
 arch/arm/mach-omap2/cclock3xxx_data.c |    1 +
 arch/arm/mach-omap2/cclock44xx_data.c |    2 ++
 2 files changed, 3 insertions(+)

diff --git a/arch/arm/mach-omap2/cclock3xxx_data.c b/arch/arm/mach-omap2/cclock3xxx_data.c
index 45cd264..9784332 100644
--- a/arch/arm/mach-omap2/cclock3xxx_data.c
+++ b/arch/arm/mach-omap2/cclock3xxx_data.c
@@ -3540,6 +3540,7 @@ static struct omap_clk omap3xxx_clks[] = {
 	CLK(NULL,	"timer_32k_ck",	&omap_32k_fck),
 	CLK(NULL,	"timer_sys_ck",	&sys_ck),
 	CLK(NULL,	"cpufreq_ck",	&dpll1_ck),
+	CLK("483072f0.regulator-abb1",	"abb_sys_ck",	&sys_ck),
 };
 
 static const char *enable_init_clks[] = {
diff --git a/arch/arm/mach-omap2/cclock44xx_data.c b/arch/arm/mach-omap2/cclock44xx_data.c
index 88e37a4..0b7914d 100644
--- a/arch/arm/mach-omap2/cclock44xx_data.c
+++ b/arch/arm/mach-omap2/cclock44xx_data.c
@@ -1684,6 +1684,8 @@ static struct omap_clk omap44xx_clks[] = {
 	CLK("4013c000.timer",	"timer_sys_ck",	&syc_clk_div_ck),
 	CLK("4013e000.timer",	"timer_sys_ck",	&syc_clk_div_ck),
 	CLK(NULL,	"cpufreq_ck",	&dpll_mpu_ck),
+	CLK("4a307bd0.regulator-abb1",	"abb_sys_ck",	&sys_clkin_ck),
+	CLK("4a307bd8.regulator-abb2",	"abb_sys_ck",	&sys_clkin_ck),
 };
 
 int __init omap4xxx_clk_init(void)
-- 
1.7.9.5

[RFC PATCH v2 5/6] ARM: OMAP3+: ABB: introduce ABB driver

[Andrii Tseglytskyi]

From: "Andrii.Tseglytskyi" <andrii.tseglytskyi@ti.com>

Patch introduces the Adaptive Body-Bias LDO driver,
which handles LDOs voltage during OPP change routine.
Current implementation is based on patch series from
Mike Turquette:

http://marc.info/?l=linux-omap&m=134931341818379&w=2

ABB can operate in the following modes:
- Bypass mode: Activated when ABB is not required
- FBB mode: Fast Body Bias mode, used on fast OPPs
- RBB mode: Reverse Body Bias mode, used on slow OPPs

ABB transition is a part of OPP changing sequence:

Case 1. Transition From Lower OPP to Higher OPP:
- Set the new OPP voltage value
- Select the next OPP for the ABB LDO
- Change MPU/IVA/CORE frequency to the new OPP frequency

ABB mode must be changed AFTER voltage scaling and
BEFORE clock rate scaling, if OPP scales UP.

Case 2. Transition From Higher OPP to Lower OPP:
- Change MPU/IVA/CORE frequency to new OPP frequency
- Select the next OPP for the ABB LDO
- Set the new OPP voltage value

ABB mode must be changed AFTER clock rate scaling and
BEFORE voltage scaling, if OPP scales DOWN.

In current implementation ABB is handled using generic
regulator framework. Standalone OPP table is used to store
ABB mode, it has the following format in device tree:

operating-points = <
	       /* uV   ABB (0 - Bypass, 1 - FBB, 2 - RBB) */
	       880000		0
	       1060000		1
	       1250000		1
	       1260000		1
>;

Cc: Mike Turquette <mturquette@linaro.org>

Signed-off-by: Andrii.Tseglytskyi <andrii.tseglytskyi@ti.com>
Signed-off-by: Mike Turquette <mturquette@linaro.org>
---
 drivers/regulator/Kconfig         |    6 +
 drivers/regulator/Makefile        |    1 +
 drivers/regulator/abb-regulator.c |  638 +++++++++++++++++++++++++++++++++++++
 3 files changed, 645 insertions(+)
 create mode 100644 drivers/regulator/abb-regulator.c

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index a5d97ea..60c0bab 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -514,5 +514,11 @@ config REGULATOR_AS3711
 	  This driver provides support for the voltage regulators on the
 	  AS3711 PMIC
 
+config REGULATOR_ABB
+	bool "TI Adaptive Body Bias"
+	depends on (ARCH_OMAP3 || ARCH_OMAP4 || SOC_OMAP5)
+	help
+	  This driver supports the ABB voltage regulators
+
 endif
 
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 6e82503..ea39b22 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -70,6 +70,7 @@ obj-$(CONFIG_REGULATOR_WM831X) += wm831x-ldo.o
 obj-$(CONFIG_REGULATOR_WM8350) += wm8350-regulator.o
 obj-$(CONFIG_REGULATOR_WM8400) += wm8400-regulator.o
 obj-$(CONFIG_REGULATOR_WM8994) += wm8994-regulator.o
+obj-$(CONFIG_REGULATOR_ABB) += abb-regulator.o
 
 
 ccflags-$(CONFIG_REGULATOR_DEBUG) += -DDEBUG
diff --git a/drivers/regulator/abb-regulator.c b/drivers/regulator/abb-regulator.c
new file mode 100644
index 0000000..5a828e2
--- /dev/null
+++ b/drivers/regulator/abb-regulator.c
@@ -0,0 +1,638 @@
+/*
+ * OMAP Adaptive Body-Bias core
+ *
+ * Copyright (C) 2011 Texas Instruments, Inc.
+ * Mike Turquette <mturquette@ti.com>
+ *
+ * Copyright (C) 2013 Texas Instruments, Inc.
+ * Andrii Tseglytskyi <andrii.tseglytskyi@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/delay.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/of_device.h>
+#include <linux/io.h>
+#include <linux/clk.h>
+#include <linux/opp.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+#include <linux/regulator/of_regulator.h>
+
+/* NOMINAL_OPP bypasses the ABB ldo, FAST_OPP sets it to Forward Body-Bias */
+#define OMAP_ABB_NOMINAL_OPP	0
+#define OMAP_ABB_FAST_OPP	1
+#define OMAP_ABB_SLOW_OPP	3
+#define OMAP_ABB_NO_LDO		(~0)
+
+/* Time for the ABB ldo to settle after transition (in micro-seconds) */
+#define ABB_TRANXDONE_TIMEOUT	50
+
+#define ABB_OPP_SEL_MASK		(0x3 << 0)
+#define ABB_OPP_CHANGE_MASK		(0x1 << 2)
+#define ABB_SR2EN_MASK			(0x1 << 0)
+#define ABB_ACTIVE_FBB_SEL_MASK		(0x1 << 2)
+#define ABB_ACTIVE_RBB_SEL_MASK		(0x1 << 1)
+#define ABB_SR2_WTCNT_VALUE_MASK	(0xff << 8)
+
+/*
+ * struct omap_abb_data - common data for each instance of ABB ldo
+ *
+ * @opp_sel_mask:	selects Fast/Nominal/Slow OPP for ABB
+ * @opp_change_mask:	selects OPP_CHANGE bit value
+ * @sr2_wtcnt_value_mask:	LDO settling time for active-mode OPP change
+ * @sr2en_mask:			enables/disables ABB
+ * @fbb_sel_mask:		selects FBB mode
+ * @rbb_sel_mask:		selects RBB mode
+ * @settling_time:	IRQ handle used to resolve IRQSTATUS offset & masks
+ * @clock_cycles:	value needed for LDO setting time calculation
+ * @setup_offs:		PRM_LDO_ABB_XXX_SETUP register offset
+ * @control_offs:	PRM_LDO_ABB_IVA_CTRL register offset
+ */
+struct omap_abb_data {
+	u32 opp_sel_mask;
+	u32 opp_change_mask;
+	u32 sr2_wtcnt_value_mask;
+	u32 sr2en_mask;
+	u32 fbb_sel_mask;
+	u32 rbb_sel_mask;
+	unsigned long settling_time;
+	unsigned long clock_cycles;
+	u8 setup_offs;
+	u8 control_offs;
+};
+
+/*
+ * struct omap_abb - ABB ldo instance
+ *
+ * @control:	memory mapped ABB registers
+ * @txdone:	memory mapped IRQSTATUS register
+ * @dev:	device, for which ABB is created
+ * @txdone_mask:	ABB mode change done bit
+ * @opp_sel:		current ABB status - Fast/Nominal/Slow
+ * @notify_clk:		clock, which rate changes are handled by ABB
+ * @data:		common data
+ * @abb_clk_nb:		clock rate change notifier block
+ */
+struct omap_abb {
+	void __iomem	*control;
+	void __iomem	*txdone;
+	struct device	*dev;
+	u32		txdone_mask;
+	u32		opp_sel;
+	u32		current_volt;
+	struct omap_abb_data	data;
+	struct regulator	*supply_reg;
+	struct regulator_desc   rdesc;
+};
+
+static const struct omap_abb_data __initdata omap36xx_abb_data = {
+	.opp_sel_mask		= ABB_OPP_SEL_MASK,
+	.opp_change_mask	= ABB_OPP_CHANGE_MASK,
+	.sr2en_mask		= ABB_SR2EN_MASK,
+	.fbb_sel_mask		= ABB_ACTIVE_FBB_SEL_MASK,
+	.sr2_wtcnt_value_mask	= ABB_SR2_WTCNT_VALUE_MASK,
+	.setup_offs		= 0,
+	.control_offs		= 0x4,
+	.settling_time		= 30,
+	.clock_cycles		= 8,
+};
+
+static const struct omap_abb_data __initdata omap4_abb_data = {
+	.opp_sel_mask		= ABB_OPP_SEL_MASK,
+	.opp_change_mask	= ABB_OPP_CHANGE_MASK,
+	.sr2en_mask		= ABB_SR2EN_MASK,
+	.fbb_sel_mask		= ABB_ACTIVE_FBB_SEL_MASK,
+	.rbb_sel_mask		= ABB_ACTIVE_RBB_SEL_MASK,
+	.sr2_wtcnt_value_mask	= ABB_SR2_WTCNT_VALUE_MASK,
+	.setup_offs		= 0,
+	.control_offs		= 0x4,
+	.settling_time		= 50,
+	.clock_cycles		= 16,
+};
+
+static const struct omap_abb_data __initdata omap5_abb_data = {
+	.opp_sel_mask		= ABB_OPP_SEL_MASK,
+	.opp_change_mask	= ABB_OPP_CHANGE_MASK,
+	.sr2en_mask		= ABB_SR2EN_MASK,
+	.fbb_sel_mask		= ABB_ACTIVE_FBB_SEL_MASK,
+	.sr2_wtcnt_value_mask	= ABB_SR2_WTCNT_VALUE_MASK,
+	.setup_offs		= 0,
+	.control_offs		= 0x4,
+	.settling_time		= 50,
+	.clock_cycles		= 16,
+};
+
+/**
+ * omap_abb_readl() - reads ABB control memory
+ * @abb:	pointer to the abb instance
+ * @offs:	offset to read
+ *
+ * Returns @offs value
+ */
+static u32 omap_abb_readl(const struct omap_abb *abb, u32 offs)
+{
+	return readl(abb->control + offs);
+}
+
+/**
+ * omap_abb_rmw() - modifies ABB control memory
+ * @abb:	pointer to the abb instance
+ * @mask:	mask to modify
+ * @bits:	bits to store
+ * @offs:	offset to modify
+ */
+static void omap_abb_rmw(const struct omap_abb *abb,
+			 u32 mask, u32 bits, u32 offs)
+{
+	u32 val;
+
+	val = readl(abb->control + offs);
+	val &= ~mask;
+	val |= bits;
+	writel(val, abb->control + offs);
+}
+
+/**
+ * omap_abb_check_txdone() - checks ABB tranxdone status
+ * @abb:	pointer to the abb instance
+ *
+ * Returns true or false
+ */
+static bool omap_abb_check_txdone(const struct omap_abb *abb)
+{
+	return !!(readl(abb->txdone) & abb->txdone_mask);
+}
+
+/**
+ * omap_abb_clear_txdone() - clears ABB tranxdone status
+ * @abb:	pointer to the abb instance
+ */
+static void omap_abb_clear_txdone(const struct omap_abb *abb)
+{
+	writel(abb->txdone_mask, abb->txdone);
+};
+
+/**
+ * omap_abb_wait_tranx() - waits for ABB tranxdone event
+ * @abb:	pointer to the abb instance
+ *
+ * Returns 0 on success or -ETIMEDOUT if the event
+ * is not set on time.
+ */
+static int omap_abb_wait_tranx(const struct omap_abb *abb)
+{
+	int timeout;
+	bool status;
+
+	timeout = 0;
+	while (timeout++ < ABB_TRANXDONE_TIMEOUT) {
+		status = omap_abb_check_txdone(abb);
+		if (status)
+			break;
+
+		udelay(1);
+	}
+
+	if (timeout >= ABB_TRANXDONE_TIMEOUT) {
+		dev_warn(abb->dev, "%s: ABB TRANXDONE timeout=(%d)\n",
+			 __func__, timeout);
+		return -ETIMEDOUT;
+	}
+	return 0;
+}
+
+/**
+ * omap_abb_clear_tranx() - clears ABB tranxdone event
+ * @abb:	pointer to the abb instance
+ *
+ * Returns 0 on success or -ETIMEDOUT if the event
+ * is not cleared on time.
+ */
+static int omap_abb_clear_tranx(const struct omap_abb *abb)
+{
+	int timeout;
+	bool status;
+
+	/* clear interrupt status */
+	timeout = 0;
+	while (timeout++ < ABB_TRANXDONE_TIMEOUT) {
+		omap_abb_clear_txdone(abb);
+
+		status = omap_abb_check_txdone(abb);
+		if (!status)
+			break;
+
+		udelay(1);
+	}
+
+	if (timeout >= ABB_TRANXDONE_TIMEOUT) {
+		dev_warn(abb->dev, "%s: ABB TRANXDONE timeout=(%d)\n",
+			 __func__, timeout);
+		return -ETIMEDOUT;
+	}
+	return 0;
+}
+
+/**
+ * omap_abb_set_opp() - program ABB ldo
+ * @abb:	pointer to the abb instance
+ * @volt:	target voltage
+ *
+ * Program the ABB ldo to the new state (if necessary), clearing the
+ * PRM_IRQSTATUS bit before and after the transition.  Returns 0 on
+ * success, -ETIMEDOUT otherwise.
+ */
+static int omap_abb_set_opp(struct omap_abb *abb, int volt)
+{
+	u32 opp_sel = 0;
+	struct opp *opp = NULL;
+	const struct omap_abb_data *data = &abb->data;
+	int ret = 0;
+
+	/* Use common OPP API to retrieve ABB sel */
+	rcu_read_lock();
+	opp = opp_find_freq_exact(abb->dev, volt * 1000, true);
+	rcu_read_unlock();
+
+	if (IS_ERR(opp)) {
+		ret = PTR_ERR(opp);
+		goto out;
+	}
+
+	opp_sel = opp_get_voltage(opp);
+
+	/* bail early if no transition is necessary */
+	if (opp_sel == abb->opp_sel)
+		goto out;
+
+	/* clear interrupt status */
+	ret = omap_abb_clear_tranx(abb);
+	if (ret)
+		goto out;
+
+	/* program the setup register */
+	switch (opp_sel) {
+	case OMAP_ABB_NOMINAL_OPP:
+		omap_abb_rmw(abb,
+			     data->fbb_sel_mask | data->rbb_sel_mask,
+			     0x0,
+			     data->setup_offs);
+		break;
+	case OMAP_ABB_SLOW_OPP:
+		omap_abb_rmw(abb,
+			     data->fbb_sel_mask | data->rbb_sel_mask,
+			     data->rbb_sel_mask,
+			     data->setup_offs);
+		break;
+	case OMAP_ABB_FAST_OPP:
+		omap_abb_rmw(abb,
+			     data->fbb_sel_mask | data->rbb_sel_mask,
+			     data->fbb_sel_mask,
+			     data->setup_offs);
+		break;
+	default:
+		/* Should have never been here! */
+		WARN_ONCE(1, "%s: opp_sel %d!!!\n",
+			  __func__, opp_sel);
+		ret = -EINVAL;
+		goto out;
+	}
+
+	/* program next state of ABB ldo */
+	omap_abb_rmw(abb, data->opp_sel_mask,
+		     opp_sel << __ffs(data->opp_sel_mask),
+		     data->control_offs);
+
+	/* initiate ABB ldo change */
+	omap_abb_rmw(abb, data->opp_change_mask,
+		     data->opp_change_mask,
+		     data->control_offs);
+
+	/* Wait for conversion completion */
+	ret = omap_abb_wait_tranx(abb);
+	if (ret)
+		goto out;
+
+	/* clear interrupt status */
+	ret = omap_abb_clear_tranx(abb);
+	if (ret)
+		goto out;
+
+	/* track internal state */
+	abb->opp_sel = opp_sel;
+
+out:
+	if (ret)
+		dev_warn(abb->dev, "%s: failed to scale: opp_sel=%d (%d)\n",
+			 __func__, opp_sel, ret);
+
+	return ret;
+}
+
+/**
+ * omap_abb_set_supplier_voltage() - scales supplier voltage
+ * @abb:	pointer to the ABB instance
+ * @min_uv:	target minimum voltage
+ * @max_uv:	target maximum voltage
+ *
+ * Returns 0 on success, or error code otherwise.
+ */
+static int omap_abb_set_supplier_voltage(const struct omap_abb *abb,
+					 int min_uv, int max_uv)
+{
+	int ret = 0;
+
+	if (!abb->supply_reg)
+		return ret;
+
+	ret = regulator_set_voltage(abb->supply_reg, min_uv, max_uv);
+	if (ret)
+		dev_err(abb->dev,
+			"%s: failed to scale supply ret (%d)\n",
+			__func__, ret);
+
+	return ret;
+}
+
+/**
+ * omap_abb_reg_set_voltage() - ABB regulator "set_voltage" callback
+ * @rdev:	ABB regulator device
+ * @min_uv:	target minimum voltage
+ * @max_uv:	target maximum voltage
+ * @selector:	unused
+ *
+ * Program the ABB ldo according to new target voltage
+ * Returns 0 on success, or error code otherwise.
+ */
+static int omap_abb_reg_set_voltage(struct regulator_dev *rdev, int min_uv,
+				    int max_uv, unsigned *selector)
+{
+	int ret = 0;
+	struct omap_abb *abb = rdev_get_drvdata(rdev);
+
+	if (abb->current_volt == min_uv)
+		return 0;
+
+	/*
+	 * handle chain regulator
+	 *
+	 * 1) if OPP scales from low to high order of scaling is the following:
+	 * - scale voltage
+	 * - set ABB mode
+	 * - scale rate
+	 *
+	 * For this function it means, if regulator is linked to chain,
+	 * supplier (which is normally AVS) need to be scaled first.
+	 * Sequence will look like
+	 *
+	 * AVS -> Voltage scale -> ABB set mode -> Freq scale
+	 *
+	 * 2) if OPP scales from high to low order is opposite:
+	 *
+	 * Freq scale -> ABB set mode -> AVS -> Voltage scale
+	 */
+	if (min_uv > abb->current_volt) {
+		ret |= omap_abb_set_supplier_voltage(abb, min_uv, max_uv);
+		ret |= omap_abb_set_opp(abb, min_uv);
+	} else {
+		ret |= omap_abb_set_opp(abb, min_uv);
+		ret |= omap_abb_set_supplier_voltage(abb, min_uv, max_uv);
+	}
+
+	if (ret)
+		dev_err(rdev_get_dev(rdev),
+			"%s: error (%d) min_uv (%d) max_uv (%d)\n",
+			__func__, ret, min_uv, max_uv);
+	else
+		abb->current_volt = min_uv;
+
+	return ret;
+}
+
+/**
+ * omap_abb_reg_get_voltage() - ABB regulator "get_voltage" callback
+ * @rdev:	ABB regulator device
+ *
+ * Returns current voltage of ABB regulator
+ */
+static int omap_abb_reg_get_voltage(struct regulator_dev *rdev)
+{
+	struct omap_abb *abb = rdev_get_drvdata(rdev);
+	return abb->current_volt;
+}
+
+static const struct of_device_id __initdata omap_abb_of_match[] = {
+	{ .compatible = "ti,omap36xx-abb", .data = &omap36xx_abb_data},
+	{ .compatible = "ti,omap4-abb", .data = &omap4_abb_data},
+	{ .compatible = "ti,omap5-abb", .data = &omap5_abb_data},
+	{},
+};
+MODULE_DEVICE_TABLE(of, omap_abb_of_match);
+
+static struct regulator_ops omap_abb_reg_ops = {
+	.set_voltage	= omap_abb_reg_set_voltage,
+	.get_voltage	= omap_abb_reg_get_voltage,
+};
+
+/*
+ * omap_abb_init_timings() - Initialize ABB timings
+ * @abb:	pointer to the ABB instance
+ *
+ * Returns 0 on success or error code otherwise
+ */
+static int __init omap_abb_init_timings(struct omap_abb *abb)
+{
+	struct clk *sys_clk = NULL;
+	u32 sys_clk_rate, sr2_wt_cnt_val, clock_cycles, abb_sel;
+
+	/*
+	 * SR2_WTCNT_VALUE is the settling time for the ABB ldo after a
+	 * transition and must be programmed with the correct time at boot.
+	 * The value programmed into the register is the number of SYS_CLK
+	 * clock cycles that match a given wall time profiled for the ldo.
+	 * This value depends on:
+	 * settling time of ldo in micro-seconds (varies per OMAP family)
+	 * # of clock cycles per SYS_CLK period (varies per OMAP family)
+	 * the SYS_CLK frequency in MHz (varies per board)
+	 * The formula is:
+	 *
+	 *                      ldo settling time (in micro-seconds)
+	 * SR2_WTCNT_VALUE = ------------------------------------------
+	 *                   (# system clock cycles) * (sys_clk period)
+	 *
+	 * Put another way:
+	 *
+	 * SR2_WTCNT_VALUE = settling time / (# SYS_CLK cycles / SYS_CLK rate))
+	 *
+	 * To avoid dividing by zero multiply both "# clock cycles" and
+	 * "settling time" by 10 such that the final result is the one we want.
+	 */
+
+	sys_clk = clk_get(abb->dev, "abb_sys_ck");
+	if (IS_ERR_OR_NULL(sys_clk))
+		return -ENODEV;
+
+	/* convert SYS_CLK rate to MHz & prevent divide by zero */
+	sys_clk_rate = DIV_ROUND_CLOSEST(clk_get_rate(sys_clk), 1000000);
+
+	/* calculate cycle rate */
+	clock_cycles = DIV_ROUND_CLOSEST((abb->data.clock_cycles * 10),
+					 sys_clk_rate);
+
+	/* calulate SR2_WTCNT_VALUE */
+	sr2_wt_cnt_val = DIV_ROUND_CLOSEST((abb->data.settling_time * 10),
+					   clock_cycles);
+
+	omap_abb_rmw(abb, abb->data.sr2_wtcnt_value_mask,
+		     (sr2_wt_cnt_val << __ffs(abb->data.sr2_wtcnt_value_mask)),
+		     abb->data.setup_offs);
+
+	/* did bootloader set OPP_SEL? */
+	abb_sel = omap_abb_readl(abb, abb->data.control_offs);
+	abb_sel &= abb->data.opp_sel_mask;
+	abb->opp_sel = abb_sel >> __ffs(abb->data.opp_sel_mask);
+
+	/* enable the ldo if not done by bootloader */
+	abb_sel = omap_abb_readl(abb, abb->data.setup_offs);
+	abb_sel &= abb->data.sr2en_mask;
+	if (!abb_sel)
+		omap_abb_rmw(abb, abb->data.sr2en_mask,
+			     abb->data.sr2en_mask, abb->data.setup_offs);
+
+	clk_put(sys_clk);
+	return 0;
+}
+
+/*
+ * omap_abb_probe() - Initialize an ABB ldo instance
+ * @pdev: ABB platform device
+ *
+ * Initializes an individual ABB ldo for Forward Body-Bias.  FBB is used to
+ * insure stability at higher voltages.  Note that some older OMAP chips have a
+ * Reverse Body-Bias mode meant to save power at low voltage, but that mode is
+ * unsupported and phased out on newer chips.
+ */
+static int __init omap_abb_probe(struct platform_device *pdev)
+{
+	const struct of_device_id *match = NULL;
+	struct omap_abb *abb = NULL;
+	struct resource *mem = NULL;
+	struct regulator_init_data *initdata = NULL;
+	struct regulator_dev *rdev = NULL;
+	struct regulator_config	config = { };
+	int ret = 0;
+
+	match = of_match_device(omap_abb_of_match, &pdev->dev);
+	if (!match) {
+		ret = -ENODEV;
+		goto err;
+	}
+
+	abb = devm_kzalloc(&pdev->dev,
+			   sizeof(struct omap_abb),
+			   GFP_KERNEL);
+	if (!abb) {
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	abb->data = *((struct omap_abb_data *)match->data);
+	abb->dev = &pdev->dev;
+
+	/* map ABB resources */
+	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!mem) {
+		ret = -ENODEV;
+		goto err;
+	}
+
+	abb->control = devm_request_and_ioremap(&pdev->dev, mem);
+	if (!abb->control) {
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	mem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	if (!mem) {
+		ret = -ENODEV;
+		goto err;
+	}
+
+	abb->txdone = devm_ioremap_nocache(&pdev->dev, mem->start,
+					   resource_size(mem));
+	if (!abb->txdone) {
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	/* read device tree properties */
+	ret = of_property_read_u32(pdev->dev.of_node,
+				   "ti,tranxdone_status_mask",
+				   &abb->txdone_mask);
+	if (ret)
+		goto err;
+
+	/* init own OPP table for ABB */
+	ret = of_init_opp_table(&pdev->dev);
+	if (ret)
+		goto err;
+
+	initdata = of_get_regulator_init_data(&pdev->dev, pdev->dev.of_node);
+	if (!initdata)
+		goto err;
+
+	/* create ABB regulator */
+	abb->rdesc.name = dev_name(&pdev->dev);
+	abb->rdesc.type = REGULATOR_VOLTAGE;
+	abb->rdesc.ops = &omap_abb_reg_ops;
+	abb->rdesc.owner = THIS_MODULE;
+
+	config.init_data = initdata;
+	config.dev = &pdev->dev;
+	config.driver_data = abb;
+	config.of_node = pdev->dev.of_node;
+
+	rdev = regulator_register(&abb->rdesc, &config);
+	if (IS_ERR(rdev)) {
+		dev_err(&pdev->dev, "failed to register regulator %s\n",
+			abb->rdesc.name);
+		ret = PTR_ERR(rdev);
+		goto err;
+	}
+
+	/* init ABB time cycles */
+	ret = omap_abb_init_timings(abb);
+	if (ret)
+		goto err;
+
+	/* get supply regulator */
+	abb->supply_reg = regulator_get(&pdev->dev, "avs");
+	if (IS_ERR(abb->supply_reg))
+		abb->supply_reg = NULL;
+
+	return 0;
+
+err:
+	dev_err(&pdev->dev, "%s: error on init (%d)\n",
+		__func__, ret);
+
+	return ret;
+}
+
+static struct platform_driver omap_abb_driver = {
+	.driver		= {
+		.name	= "omap_abb",
+		.of_match_table = of_match_ptr(omap_abb_of_match),
+	},
+};
+
+static int __init omap_abb_driver_init(void)
+{
+	return platform_driver_probe(&omap_abb_driver, omap_abb_probe);
+}
+subsys_initcall(omap_abb_driver_init);
-- 
1.7.9.5

Re: [RFC PATCH v2 5/6] ARM: OMAP3+: ABB: introduce ABB driver

[Mike Turquette]

Quoting Andrii Tseglytskyi (2013-04-15 06:28:10)
> Cc: Mike Turquette <mturquette@linaro.org>
> 
> Signed-off-by: Andrii.Tseglytskyi <andrii.tseglytskyi@ti.com>
> Signed-off-by: Mike Turquette <mturquette@linaro.org>

It is very strange to Cc me and include my Signed-off-by.  Go ahead and
drop my SoB.  I'll review these patches this week but I don't think
implicitly including my SoB is appropriate.

Also why is this limited to LOML?  Cc LKML and Mark Brown (regulator
maintainer) on the next version.  More eyeballs on the code is better.

Finally I also suggest that you rename this patch to:

"regulator: introduce omap abb driver"

As we move stuff out of arch/arm it's important to loop in wider mailing
lists, Cc the appropriate maintainers and title patches more
appropriately.

Regards,
Mike

Re: [RFC PATCH v2 5/6] ARM: OMAP3+: ABB: introduce ABB driver

[Andrii Tseglytskyi]

Hi Mike,

On 04/15/2013 07:43 PM, Mike Turquette wrote:
> Quoting Andrii Tseglytskyi (2013-04-15 06:28:10)
>> Cc: Mike Turquette <mturquette@linaro.org>
>>
>> Signed-off-by: Andrii.Tseglytskyi <andrii.tseglytskyi@ti.com>
>> Signed-off-by: Mike Turquette <mturquette@linaro.org>
> It is very strange to Cc me and include my Signed-off-by.  Go ahead and
> drop my SoB.  I'll review these patches this week but I don't think
> implicitly including my SoB is appropriate.
OK. I'll drop your Signed-off-by

> Also why is this limited to LOML?  Cc LKML and Mark Brown (regulator
> maintainer) on the next version.  More eyeballs on the code is better.
>
> Finally I also suggest that you rename this patch to:
>
> "regulator: introduce omap abb driver"
OK.

> As we move stuff out of arch/arm it's important to loop in wider mailing
> lists, Cc the appropriate maintainers and title patches more
> appropriately.
I'm going to post next patch series to wider mailing lists.
Before doing this I prefer to finish at least one review cycle with you.

Thank you.

Regards,
Andrii

[RFC PATCH v2 6/6] ARM: OMAP3+: ABB: introduce debugfs entry

[Andrii Tseglytskyi]

From: "Andrii.Tseglytskyi" <andrii.tseglytskyi@ti.com>

Patch adds debugfs entry for each ABB:
/sys/kernel/debug/<ABB device name>

This entry is read-only. It prints current state of ABB.

Signed-off-by: Andrii.Tseglytskyi <andrii.tseglytskyi@ti.com>
---
 drivers/regulator/abb-regulator.c |   74 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 73 insertions(+), 1 deletion(-)

diff --git a/drivers/regulator/abb-regulator.c b/drivers/regulator/abb-regulator.c
index 5a828e2..dad517d 100644
--- a/drivers/regulator/abb-regulator.c
+++ b/drivers/regulator/abb-regulator.c
@@ -19,6 +19,7 @@
 #include <linux/io.h>
 #include <linux/clk.h>
 #include <linux/opp.h>
+#include <linux/debugfs.h>
 #include <linux/regulator/driver.h>
 #include <linux/regulator/machine.h>
 #include <linux/regulator/of_regulator.h>
@@ -440,6 +441,74 @@ static struct regulator_ops omap_abb_reg_ops = {
 };
 
 /*
+ * omap_abb_info_dump() - ABB debug dump
+ *
+ * Returns 0
+ */
+static int omap_abb_info_dump(struct seq_file *sf, void *unused)
+{
+	const struct omap_abb *abb = (struct omap_abb *)sf->private;
+	struct opp *opp;
+	u32 abb_ctrl, abb_setup;
+	unsigned long freq = 0;
+
+	if (!abb) {
+		seq_printf(sf, "No ABB defined\n");
+		goto err;
+	}
+
+	abb_ctrl =  omap_abb_readl(abb, abb->data.control_offs);
+	abb_setup = omap_abb_readl(abb, abb->data.setup_offs);
+	seq_printf(sf, "Enabled\t->\t%d\n"
+		   "opp_sel\t->\t%u\n"
+		   "FBB mode\t->\t%u\n"
+		   "RBB mode\t->\t%u\n"
+		   "PRM_LDO_ABB_XXX_SETUP\t->\t0x%08x\n"
+		   "PRM_LDO_ABB_XXX_CTRL\t->\t0x%08x\n",
+		   !!(abb_setup & abb->data.sr2en_mask),
+		   abb->opp_sel,
+		   !!(abb_setup & abb->data.fbb_sel_mask),
+		   !!(abb_setup & abb->data.rbb_sel_mask),
+		   abb_setup,
+		   abb_ctrl);
+
+	seq_printf(sf, "OPP table\n");
+	rcu_read_lock();
+	do {
+		opp = opp_find_freq_ceil(abb->dev, &freq);
+		if (IS_ERR(opp))
+			break;
+
+		seq_printf(sf, "Voltage (%lu) ABB (%lu)\n",
+			   opp_get_freq(opp) / 1000,
+			   opp_get_voltage(opp));
+
+		freq++;
+	} while (1);
+	rcu_read_unlock();
+
+err:
+	return 0;
+}
+
+/*
+ * omap_abb_fops_open() - debugfs "open" callback
+ *
+ * Returns 0 on success or error code otherwise
+ */
+static int omap_abb_fops_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, omap_abb_info_dump, inode->i_private);
+}
+
+static const struct file_operations omap_abb_debug_fops = {
+	.open = omap_abb_fops_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = single_release,
+};
+
+/*
  * omap_abb_init_timings() - Initialize ABB timings
  * @abb:	pointer to the ABB instance
  *
@@ -507,7 +576,6 @@ static int __init omap_abb_init_timings(struct omap_abb *abb)
 	clk_put(sys_clk);
 	return 0;
 }
-
 /*
  * omap_abb_probe() - Initialize an ABB ldo instance
  * @pdev: ABB platform device
@@ -615,6 +683,10 @@ static int __init omap_abb_probe(struct platform_device *pdev)
 	if (IS_ERR(abb->supply_reg))
 		abb->supply_reg = NULL;
 
+	/* create debugfs entry */
+	debugfs_create_file(dev_name(&pdev->dev), S_IRUGO, NULL,
+			    abb, &omap_abb_debug_fops);
+
 	return 0;
 
 err:
-- 
1.7.9.5

Re: [RFC PATCH v2 0/6] ARM: OMAP3+: Introduce ABB driver

[Kevin Hilman]

Andrii Tseglytskyi <andrii.tseglytskyi@ti.com> writes:

> From: "Andrii.Tseglytskyi" <andrii.tseglytskyi@ti.com>
>
> Following patch series introduces the Adaptive Body-Bias
> LDO driver, which handles LDOs voltage during OPP change routine.
> Current implementation is based on patch series from
> Mike Turquette:
>
> http://marc.info/?l=linux-omap&m=134931341818379&w=2
>
> ABB transition is a part of OPP changing sequence.
> ABB can operate in the following modes:
> - Bypass mode: Activated when ABB is not required
> - FBB mode: Fast Body Bias mode, used on fast OPPs

Fast?  I thought the 'F' was for Forward?

> - RBB mode: Reverse Body Bias mode, used on slow OPPs
>
> In current implementation ABB is converted to regulator.
> Standalone OPP table is used to store ABB mode, it is defined
> in device tree for each ABB regulator. It has the following format:
>
> operating-points = <
> 	       /* uV   ABB (0 - Bypass, 1 - FBB, 2 - RBB) */
> 	       880000		0
> 	       1060000		1
> 	       1250000		1
> 	       1260000		1
>>;
>
> ABB regulator is linked to regulator chain

In addition to Mike's comments (which I completely agree with), it would
be very helfpul to see how this is actually used.  e.g, how the
regulators are chained together, how the proper ordering is managed,
etc. etc.

IOW, This series gives a bunch of low-level details without
demonstrating the actual use case and showing the regulator API usage
that would make this work.

Kevin

Re: [RFC PATCH v2 0/6] ARM: OMAP3+: Introduce ABB driver

[Andrii Tseglytskyi]

Hi Kevin,

On 04/16/2013 12:53 AM, Kevin Hilman wrote:
> Andrii Tseglytskyi <andrii.tseglytskyi@ti.com> writes:
>
>> From: "Andrii.Tseglytskyi" <andrii.tseglytskyi@ti.com>
>>
>> Following patch series introduces the Adaptive Body-Bias
>> LDO driver, which handles LDOs voltage during OPP change routine.
>> Current implementation is based on patch series from
>> Mike Turquette:
>>
>> http://marc.info/?l=linux-omap&m=134931341818379&w=2
>>
>> ABB transition is a part of OPP changing sequence.
>> ABB can operate in the following modes:
>> - Bypass mode: Activated when ABB is not required
>> - FBB mode: Fast Body Bias mode, used on fast OPPs
> Fast?  I thought the 'F' was for Forward?
You are right. Should be 'Forward' here.

>> - RBB mode: Reverse Body Bias mode, used on slow OPPs
>>
>> In current implementation ABB is converted to regulator.
>> Standalone OPP table is used to store ABB mode, it is defined
>> in device tree for each ABB regulator. It has the following format:
>>
>> operating-points = <
>> 	       /* uV   ABB (0 - Bypass, 1 - FBB, 2 - RBB) */
>> 	       880000		0
>> 	       1060000		1
>> 	       1250000		1
>> 	       1260000		1
>>> ;
>> ABB regulator is linked to regulator chain
> In addition to Mike's comments (which I completely agree with), it would
> be very helfpul to see how this is actually used.  e.g, how the
> regulators are chained together, how the proper ordering is managed,
> etc. etc.

We would like to handle voltage scaling in the following way:

cpufreq_cpu0
clk_set_rate(cpu0)
     |
     |-->set_voltage(ABB regulator) /* all ABB related stuff will be 
handled here */
                 |
                 |-->set_voltage(smps123 regulator) /* actual voltage 
scaling */


This simple model will be extended to handle AVS as a part of the chain.
smps123 regulator may be changed to VP/VC regulator.

Following example is from integration branch, which already has smps123 
regulator.
It demonstrates an example of linkage to chain. ABB regulator is linked 
with smps123 and cpu0 inside device tree.
cpu0 calls set_voltage() function for ABB, and then ABB calls 
set_voltage() function for smps123 to do actual voltage scaling.

diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
index bb5ee70..c8cbbee 100644
--- a/arch/arm/boot/dts/omap5.dtsi
+++ b/arch/arm/boot/dts/omap5.dtsi
@@ -36,7 +36,7 @@
         cpus {
                 cpu@0 {
                         compatible = "arm,cortex-a15";
-                       cpu0-supply = <&smps123_reg>;
+                       cpu0-supply = <&abb_mpu>;
                         operating-points = <
                                 /* kHz    uV */
                                 /* Only for Nominal Samples */
@@ -94,6 +94,7 @@
                 reg = <0x4ae07cdc 0x8>,
                       <0x4ae06014 0x4>;
                 ti,tranxdone_status_mask = <0x80>;
+               avs-supply = <&smps123_reg>;
                 operating-points = <
                         /* uV   ABB */
                         880000  0

This RFC patch series is verified together with:
https://patchwork.kernel.org/patch/2445091/

Kevin, what do you think about this model in general? Does it fit to 
regulator framework?

Thank you.

Regards,
Andrii

Re: [RFC PATCH v2 0/6] ARM: OMAP3+: Introduce ABB driver

[Mike Turquette]

Quoting Andrii Tseglytskyi (2013-04-16 05:40:44)
> On 04/16/2013 12:53 AM, Kevin Hilman wrote:
> > In addition to Mike's comments (which I completely agree with), it would
> > be very helfpul to see how this is actually used.  e.g, how the
> > regulators are chained together, how the proper ordering is managed,
> > etc. etc.
> 
> We would like to handle voltage scaling in the following way:
> 

I expanded the example below to include the SR AVS regulator.

> cpufreq_cpu0
> clk_set_rate(cpu0)
>      |
>      |-->set_voltage(ABB regulator)
>                  |
>                  |-->set_voltage(AVS)
>                             |
>                             |-->set_voltage(smps123 regulator)

Hi Andrii,

Why was regulator chaining chosen over a simple sequence of calls to
regulator_set_voltage?  Instead of nested calls into the regulator
framework, why don't you just make the calls serially?  E.g:

regulator_set_voltage(abb_reg, foo_volt);
regulator_set_voltage(avs_reg, bar_volt);
regulator_set_voltage(smps123_reg, baz_volt);

It is still to be determined where these calls originate from; maybe
from clock notifiers, maybe directly from the cpufreq driver's .target()
callback, or maybe somewhere else.  Regardless, I do not see why
regulator chaining is truly necessary here.  You are just calling
regulator_set_voltage in sequence on a few regulators, right?

I think it would help me a lot to understand why regulator chaining is a
requirement for this to work properly.

Thanks,
Mike

> 
> 
> This simple model will be extended to handle AVS as a part of the chain.
> smps123 regulator may be changed to VP/VC regulator.
> 
> Following example is from integration branch, which already has smps123 
> regulator.
> It demonstrates an example of linkage to chain. ABB regulator is linked 
> with smps123 and cpu0 inside device tree.
> cpu0 calls set_voltage() function for ABB, and then ABB calls 
> set_voltage() function for smps123 to do actual voltage scaling.
> 
> diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
> index bb5ee70..c8cbbee 100644
> --- a/arch/arm/boot/dts/omap5.dtsi
> +++ b/arch/arm/boot/dts/omap5.dtsi
> @@ -36,7 +36,7 @@
>          cpus {
>                  cpu@0 {
>                          compatible = "arm,cortex-a15";
> -                       cpu0-supply = <&smps123_reg>;
> +                       cpu0-supply = <&abb_mpu>;
>                          operating-points = <
>                                  /* kHz    uV */
>                                  /* Only for Nominal Samples */
> @@ -94,6 +94,7 @@
>                  reg = <0x4ae07cdc 0x8>,
>                        <0x4ae06014 0x4>;
>                  ti,tranxdone_status_mask = <0x80>;
> +               avs-supply = <&smps123_reg>;
>                  operating-points = <
>                          /* uV   ABB */
>                          880000  0
> 
> This RFC patch series is verified together with:
> https://patchwork.kernel.org/patch/2445091/
> 
> Kevin, what do you think about this model in general? Does it fit to 
> regulator framework?
> 
> Thank you.
> 
> Regards,
> Andrii

Re: [RFC PATCH v2 0/6] ARM: OMAP3+: Introduce ABB driver

[Grygorii Strashko]

On 04/16/2013 10:07 PM, Mike Turquette wrote:
> Quoting Andrii Tseglytskyi (2013-04-16 05:40:44)
>> On 04/16/2013 12:53 AM, Kevin Hilman wrote:
>>> In addition to Mike's comments (which I completely agree with), it would
>>> be very helfpul to see how this is actually used.  e.g, how the
>>> regulators are chained together, how the proper ordering is managed,
>>> etc. etc.
>> We would like to handle voltage scaling in the following way:
>>
> I expanded the example below to include the SR AVS regulator.
>
>> cpufreq_cpu0
>> clk_set_rate(cpu0)
>>       |
>>       |-->set_voltage(ABB regulator)
>>                   |
>>                   |-->set_voltage(AVS)
>>                              |
>>                              |-->set_voltage(smps123 regulator)
> Hi Andrii,
>
> Why was regulator chaining chosen over a simple sequence of calls to
> regulator_set_voltage?  Instead of nested calls into the regulator
> framework, why don't you just make the calls serially?  E.g:
>
> regulator_set_voltage(abb_reg, foo_volt);
> regulator_set_voltage(avs_reg, bar_volt);
> regulator_set_voltage(smps123_reg, baz_volt);
>
> It is still to be determined where these calls originate from; maybe
> from clock notifiers, maybe directly from the cpufreq driver's .target()
> callback, or maybe somewhere else.  Regardless, I do not see why
> regulator chaining is truly necessary here.  You are just calling
> regulator_set_voltage in sequence on a few regulators, right?
>
> I think it would help me a lot to understand why regulator chaining is a
> requirement for this to work properly.
>
> Thanks,
> Mike
Hi Mike, Kevin

I'd like to provide some explanation regarding proposed TI DVFS design 
which we
would like to follow (regulator chaining is part of it).
The following two patch series was intended to prove the possibility of its
implementation:
   - this one  'ARM: OMAP3+: Introduce ABB driver"
   - and http://www.spinics.net/lists/linux-omap/msg90022.html -
      [RFC v1 0/1] introduce regulator chain locking scheme
but, seems, we started to make it public from "tail" instead of 'head"
  - sorry for that.

While trying to move forward with TI DVFS we've taken into account following
major points:
- only DT should be used to configure DVFS;
- no xxx_initcall, cpu_is_xx() function should be used;
- DVFS should be scalable  to fit wide SoC/platform’s and
   multiplatform kernel requirements;
- minimize creation of TI specific API;

Now, there are two entry points for DVFS in kernel:
- CPUFreq - currently it's been decided to use cpufreq-cpu0 for
   all OMAPs in Main line;
- CCF callbacks - have RFC DVFS implementation introduced here
   http://lwn.net/Articles/540422/.

In both cases, the only one regulator need to be provided for CCF
or CPUFreq for voltage changing proposes, so DVFS can done
in the following way:

   |------------|   |------------|
--| RegulatorY |<--|     DVFS   |
   |------------|   |------------|
    \           \
     \           \_____________________________
      \                                        \
      |-------------------|  |---------------|  |---------|
    --| RegulatorX (PMIC) |--| Regulator AVS |--| ABB LDO |--
      |-------------------|  |---------------|  |---------|
         /|\                     |
          |______________________|
          Voltage adjustment


1) The following use-cases have been taken into account:
- SoC/Platform don't need ABB/AVS (supports MPU OPPLOW/OPPNOM for example) -
   any regulator (VC-VP/I2C/SPI/GPIO ..) may be connected to DVFS
- SoC/Platform need ABB -
   build chain in DT device->CCF->abb_regulator->any 
regulator(VC-VP/I2C/SPI..)
- SoC/Platform need ABB+AVS -
   build chain in DT device->CCF->abb_regulator->avsX->any 
regulator(VC-VP/I2C..)

2) Implementation of each part of Voltage scale chain as Regulator will 
allow:
- add each item one by one;
- don't expose too much of custom TI API;
- handle multi-voltage scaling requests to one rail (ganged rails) 
automatically
  (handled by regulator FW already).

3) The "regulator chaining" will allow:
- easily configure DVFS form DT depending on current SoC/platform needs
   (using xxx-supply standard binding in DT);
- continue use cpufreq-cpu0 for all OMAP to scale MPU domain;
- use RFC DVFS implementation from http://lwn.net/Articles/540422/ for other
   domains (with some modifications - the most difficult thing will be 
multi-freq
   requests handling to one clock).

In case, if "regulator chaining" approach is not accepted:
- yes, it's possible to create some "Super" TI regulator which will
   handle ABB+AVS+VC/VP for most of current TI SoCs.

- No, for the newest TI SoCs (like DRA7xxx without VC/VP) any regulator 
can be
   used as power supply (I2C/SPI/GPIO) and ABB is needed.
   a) As result, it will be impossible to use cpufreq-cpu0 driver for it,
   at least, and will need to drop cpufreq-cpu0 support for OMAPs
   and roll-back to omap-cpufreq.

   b) for other domains it's possible to create omap_dvfs.c in the similar
   way as it done in dvfs.c and hack it to handle ABB+AVS+I2C regulator.

- will need to add custom TI bindings to handle SoC/Platform dependent
configuration;

Thanks for you time.

Regards
-grygorii
>>
>> This simple model will be extended to handle AVS as a part of the chain.
>> smps123 regulator may be changed to VP/VC regulator.
>>
>> Following example is from integration branch, which already has smps123
>> regulator.
>> It demonstrates an example of linkage to chain. ABB regulator is linked
>> with smps123 and cpu0 inside device tree.
>> cpu0 calls set_voltage() function for ABB, and then ABB calls
>> set_voltage() function for smps123 to do actual voltage scaling.
>>
>> diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
>> index bb5ee70..c8cbbee 100644
>> --- a/arch/arm/boot/dts/omap5.dtsi
>> +++ b/arch/arm/boot/dts/omap5.dtsi
>> @@ -36,7 +36,7 @@
>>           cpus {
>>                   cpu@0 {
>>                           compatible = "arm,cortex-a15";
>> -                       cpu0-supply = <&smps123_reg>;
>> +                       cpu0-supply = <&abb_mpu>;
>>                           operating-points = <
>>                                   /* kHz    uV */
>>                                   /* Only for Nominal Samples */
>> @@ -94,6 +94,7 @@
>>                   reg = <0x4ae07cdc 0x8>,
>>                         <0x4ae06014 0x4>;
>>                   ti,tranxdone_status_mask = <0x80>;
>> +               avs-supply = <&smps123_reg>;
>>                   operating-points = <
>>                           /* uV   ABB */
>>                           880000  0
>>
>> This RFC patch series is verified together with:
>> https://patchwork.kernel.org/patch/2445091/
>>
>> Kevin, what do you think about this model in general? Does it fit to
>> regulator framework?
>>
>> Thank you.
>>
>> Regards,
>> Andrii
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH v2 0/6] ARM: OMAP3+: Introduce ABB driver

[Kevin Hilman]

Andrii Tseglytskyi <andrii.tseglytskyi@ti.com> writes:

> Hi Kevin,
>
> On 04/16/2013 12:53 AM, Kevin Hilman wrote:
>> Andrii Tseglytskyi <andrii.tseglytskyi@ti.com> writes:
>>
>>> From: "Andrii.Tseglytskyi" <andrii.tseglytskyi@ti.com>
>>>
>>> Following patch series introduces the Adaptive Body-Bias
>>> LDO driver, which handles LDOs voltage during OPP change routine.
>>> Current implementation is based on patch series from
>>> Mike Turquette:
>>>
>>> http://marc.info/?l=linux-omap&m=134931341818379&w=2
>>>
>>> ABB transition is a part of OPP changing sequence.
>>> ABB can operate in the following modes:
>>> - Bypass mode: Activated when ABB is not required
>>> - FBB mode: Fast Body Bias mode, used on fast OPPs
>> Fast?  I thought the 'F' was for Forward?
> You are right. Should be 'Forward' here.
>
>>> - RBB mode: Reverse Body Bias mode, used on slow OPPs
>>>
>>> In current implementation ABB is converted to regulator.
>>> Standalone OPP table is used to store ABB mode, it is defined
>>> in device tree for each ABB regulator. It has the following format:
>>>
>>> operating-points = <
>>> 	       /* uV   ABB (0 - Bypass, 1 - FBB, 2 - RBB) */
>>> 	       880000		0
>>> 	       1060000		1
>>> 	       1250000		1
>>> 	       1260000		1
>>>> ;
>>> ABB regulator is linked to regulator chain
>> In addition to Mike's comments (which I completely agree with), it would
>> be very helfpul to see how this is actually used.  e.g, how the
>> regulators are chained together, how the proper ordering is managed,
>> etc. etc.
>
> We would like to handle voltage scaling in the following way:

What I meant is that a detailed description of the use case should be
included in the changelog.

> cpufreq_cpu0
> clk_set_rate(cpu0)
>     |
>     |-->set_voltage(ABB regulator) /* all ABB related stuff will be
> handled here */
>                 |
>                 |-->set_voltage(smps123 regulator) /* actual voltage
> scaling */

-EASCII_ART_WRAP

>
> This simple model will be extended to handle AVS as a part of the chain.
> smps123 regulator may be changed to VP/VC regulator.
>
> Following example is from integration branch, which already has
> smps123 regulator.

I don't know what integration branch you're referring to, and I don't
know what the smps123 regulator is.

> It demonstrates an example of linkage to chain. ABB regulator is
> linked with smps123 and cpu0 inside device tree.
> cpu0 calls set_voltage() function for ABB, and then ABB calls
> set_voltage() function for smps123 to do actual voltage scaling.
>
> diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
> index bb5ee70..c8cbbee 100644
> --- a/arch/arm/boot/dts/omap5.dtsi
> +++ b/arch/arm/boot/dts/omap5.dtsi
> @@ -36,7 +36,7 @@
>         cpus {
>                 cpu@0 {
>                         compatible = "arm,cortex-a15";
> -                       cpu0-supply = <&smps123_reg>;
> +                       cpu0-supply = <&abb_mpu>;
>                         operating-points = <
>                                 /* kHz    uV */
>                                 /* Only for Nominal Samples */
> @@ -94,6 +94,7 @@
>                 reg = <0x4ae07cdc 0x8>,
>                       <0x4ae06014 0x4>;
>                 ti,tranxdone_status_mask = <0x80>;
> +               avs-supply = <&smps123_reg>;
>                 operating-points = <
>                         /* uV   ABB */
>                         880000  0
>
> This RFC patch series is verified together with:
> https://patchwork.kernel.org/patch/2445091/
>
> Kevin, what do you think about this model in general? Does it fit to
> regulator framework?

I don't know yet, because I don't think the use case has been described
well enough for me to fully understand it the motiviation behind the
series.

In addition, there are alternative approaches that seem to have been
ruled out without describing why.  For example, the regulator framework
already allows you to override methods with custom hooks (as we do for
VC/VP controlled regulators already.)  Without thinking about it too
deeply, it seems this approach could be used to manage the chain of
events you need as well.  I can imagine there are limitations to this
approach for what you're trying to do, but I don't feel they have been
described in the changelog as part of the motivation for this series.

So for now, the guidance I have is this:

First, write changelogs (and cover letters) assuming your audience has
not been staring at the code as long as you have.  Even if they have
been staring at the same code, assume they've been staring at mainline,
and not a random integration branch somewhere.  My general advice is to
write changelogs in a way that you will understand what you wrote a year
from now after having forgotten all the details currently in your brains
cache.  Even better, write them so that I will understand them in a year
since I forget much better than I remember.

Second, before inventing something new, start with the existing
framework.  When the existing framework doesn't work, make an argument
for your new approach or extentions to the framework based on why the
existing stuff doesn't work.  If you don't do this, the reviewers first
reaction will almost always be "why don't you use what already exists in
the framework."  And then you'll have a bunch of back and forth with
reviewers when you could've explained the reasoning from the beginning.

Hope that helps,

Kevin

Re: [RFC PATCH v2 0/6] ARM: OMAP3+: Introduce ABB driver

[Andrii Tseglytskyi]

On 04/16/2013 10:18 PM, Kevin Hilman wrote:
> Andrii Tseglytskyi <andrii.tseglytskyi@ti.com> writes:
>
>> Hi Kevin,
>>
>> On 04/16/2013 12:53 AM, Kevin Hilman wrote:
>>> Andrii Tseglytskyi <andrii.tseglytskyi@ti.com> writes:
>>>
>>>> From: "Andrii.Tseglytskyi" <andrii.tseglytskyi@ti.com>
>>>>
>>>> Following patch series introduces the Adaptive Body-Bias
>>>> LDO driver, which handles LDOs voltage during OPP change routine.
>>>> Current implementation is based on patch series from
>>>> Mike Turquette:
>>>>
>>>> http://marc.info/?l=linux-omap&m=134931341818379&w=2
>>>>
>>>> ABB transition is a part of OPP changing sequence.
>>>> ABB can operate in the following modes:
>>>> - Bypass mode: Activated when ABB is not required
>>>> - FBB mode: Fast Body Bias mode, used on fast OPPs
>>> Fast?  I thought the 'F' was for Forward?
>> You are right. Should be 'Forward' here.
>>
>>>> - RBB mode: Reverse Body Bias mode, used on slow OPPs
>>>>
>>>> In current implementation ABB is converted to regulator.
>>>> Standalone OPP table is used to store ABB mode, it is defined
>>>> in device tree for each ABB regulator. It has the following format:
>>>>
>>>> operating-points = <
>>>> 	       /* uV   ABB (0 - Bypass, 1 - FBB, 2 - RBB) */
>>>> 	       880000		0
>>>> 	       1060000		1
>>>> 	       1250000		1
>>>> 	       1260000		1
>>>>> ;
>>>> ABB regulator is linked to regulator chain
>>> In addition to Mike's comments (which I completely agree with), it would
>>> be very helfpul to see how this is actually used.  e.g, how the
>>> regulators are chained together, how the proper ordering is managed,
>>> etc. etc.
>> We would like to handle voltage scaling in the following way:
> What I meant is that a detailed description of the use case should be
> included in the changelog.
>
>> cpufreq_cpu0
>> clk_set_rate(cpu0)
>>      |
>>      |-->set_voltage(ABB regulator) /* all ABB related stuff will be
>> handled here */
>>                  |
>>                  |-->set_voltage(smps123 regulator) /* actual voltage
>> scaling */
> -EASCII_ART_WRAP
>
>> This simple model will be extended to handle AVS as a part of the chain.
>> smps123 regulator may be changed to VP/VC regulator.
>>
>> Following example is from integration branch, which already has
>> smps123 regulator.
> I don't know what integration branch you're referring to, and I don't
> know what the smps123 regulator is.
>
>> It demonstrates an example of linkage to chain. ABB regulator is
>> linked with smps123 and cpu0 inside device tree.
>> cpu0 calls set_voltage() function for ABB, and then ABB calls
>> set_voltage() function for smps123 to do actual voltage scaling.
>>
>> diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
>> index bb5ee70..c8cbbee 100644
>> --- a/arch/arm/boot/dts/omap5.dtsi
>> +++ b/arch/arm/boot/dts/omap5.dtsi
>> @@ -36,7 +36,7 @@
>>          cpus {
>>                  cpu@0 {
>>                          compatible = "arm,cortex-a15";
>> -                       cpu0-supply = <&smps123_reg>;
>> +                       cpu0-supply = <&abb_mpu>;
>>                          operating-points = <
>>                                  /* kHz    uV */
>>                                  /* Only for Nominal Samples */
>> @@ -94,6 +94,7 @@
>>                  reg = <0x4ae07cdc 0x8>,
>>                        <0x4ae06014 0x4>;
>>                  ti,tranxdone_status_mask = <0x80>;
>> +               avs-supply = <&smps123_reg>;
>>                  operating-points = <
>>                          /* uV   ABB */
>>                          880000  0
>>
>> This RFC patch series is verified together with:
>> https://patchwork.kernel.org/patch/2445091/
>>
>> Kevin, what do you think about this model in general? Does it fit to
>> regulator framework?
> I don't know yet, because I don't think the use case has been described
> well enough for me to fully understand it the motiviation behind the
> series.
>
> In addition, there are alternative approaches that seem to have been
> ruled out without describing why.  For example, the regulator framework
> already allows you to override methods with custom hooks (as we do for
> VC/VP controlled regulators already.)  Without thinking about it too
> deeply, it seems this approach could be used to manage the chain of
> events you need as well.  I can imagine there are limitations to this
> approach for what you're trying to do, but I don't feel they have been
> described in the changelog as part of the motivation for this series.
>
> So for now, the guidance I have is this:
>
> First, write changelogs (and cover letters) assuming your audience has
> not been staring at the code as long as you have.  Even if they have
> been staring at the same code, assume they've been staring at mainline,
> and not a random integration branch somewhere.  My general advice is to
> write changelogs in a way that you will understand what you wrote a year
> from now after having forgotten all the details currently in your brains
> cache.  Even better, write them so that I will understand them in a year
> since I forget much better than I remember.
>
> Second, before inventing something new, start with the existing
> framework.  When the existing framework doesn't work, make an argument
> for your new approach or extentions to the framework based on why the
> existing stuff doesn't work.  If you don't do this, the reviewers first
> reaction will almost always be "why don't you use what already exists in
> the framework."  And then you'll have a bunch of back and forth with
> reviewers when you could've explained the reasoning from the beginning.
>
> Hope that helps,
>
> Kevin
>
>
Hi Kevin, Mike

Thanks a lot for your comments.

"regulator chain" approach was added to ABB series to demonstrate how this
approach works in general, and get more comments on this.
It was initially introduced in:

https://lkml.org/lkml/2013/4/5/33 <https://lkml.org/lkml/2013/4/5/33> - 
regulator: query on regulator re-entrance
http://www.spinics.net/lists/linux-omap/msg90022.html - [RFC v1 0/1] 
introduce regulator chain locking scheme

Taking in account your comments - I'll split ABB logic and "regulator 
chain" logic,
ABB has a possibility to work standalone. In this case DVFS framework 
will be responsible
for calls like *regulator_set_voltage(abb_regulator)*.

Could you please exclude "regulator chain" approach from this review and 
take a look to ABB handling logic?

Regards,
Andrii