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

[PATCH 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.
This data is based on OMAP36XX_ES1.x_NDA_TRM_vAB.pdf
document.

Cc: Mike Turquette <mturquette@linaro.org>
Cc: Tero Kristo <t-kristo@ti.com>
Cc: Nishanth Menon <nm@ti.com>
Cc: "Benoît Cousson" <b-cousson@ti.com>
Cc: devicetree-discuss@lists.ozlabs.org
Cc: linux-omap@vger.kernel.org

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

diff --git a/arch/arm/boot/dts/omap36xx.dtsi b/arch/arm/boot/dts/omap36xx.dtsi
index 96bf028..72d5135 100644
--- a/arch/arm/boot/dts/omap36xx.dtsi
+++ b/arch/arm/boot/dts/omap36xx.dtsi
@@ -15,6 +15,22 @@
 		serial3 = &uart4;
 	};
 
+	abb@0x483072f0 {
+		compatible = "ti,omap36xx-abb";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		reg = <0x483072f0 0x8>,
+		      <0x48306818 0x4>; /* PRM_IRQSTATUS_MPU */
+		operating-points = <
+			/* kHz   ABB */
+			300000  0
+			600000  0
+			800000  0
+			1000000 1
+		>;
+		ti,tranxdone_status_mask = <0x4000000>; /* (1 << 26) */
+	};
+
 	ocp {
 		uart4: serial@49042000 {
 			compatible = "ti,omap3-uart";
-- 
1.7.9.5

--
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

[PATCH 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.
This data is based on OMAP4430_ES2.x_NDA_TRM_vAA.pdf
document.

Cc: Mike Turquette <mturquette@linaro.org>
Cc: Tero Kristo <t-kristo@ti.com>
Cc: Nishanth Menon <nm@ti.com>
Cc: "Benoît Cousson" <b-cousson@ti.com>
Cc: devicetree-discuss@lists.ozlabs.org
Cc: linux-omap@vger.kernel.org

Signed-off-by: Andrii.Tseglytskyi <andrii.tseglytskyi@ti.com>
---
 arch/arm/boot/dts/omap4.dtsi    |   18 ++++++++++++++++++
 arch/arm/boot/dts/omap443x.dtsi |   20 ++++++++++++++++++++
 arch/arm/boot/dts/omap446x.dtsi |   36 ++++++++++++++++++++++++++++++++++++
 3 files changed, 74 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 739bb79..0bc3ef1 100644
--- a/arch/arm/boot/dts/omap4.dtsi
+++ b/arch/arm/boot/dts/omap4.dtsi
@@ -81,6 +81,24 @@
 		};
 	};
 
+	abb@0x4a307bd0 {
+		compatible = "ti,omap4-abb";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		reg = <0x4a307bd0 0x8>,
+		      <0x4a306014 0x4>; /* PRM_IRQSTATUS_MPU_A9_2 */
+		ti,tranxdone_status_mask = <0x80>; /* (1 << 7) */
+	};
+
+	abb@0x4a307bd8 {
+		compatible = "ti,omap4-abb";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		reg = <0x4a307bd8 0x8>,
+		      <0x4a306010 0x4>; /* PRM_IRQSTATUS_MPU_A9 */
+		ti,tranxdone_status_mask = <0x80000000>; /* (1 << 31) */
+	};
+
 	/*
 	 * 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..6c511c6 100644
--- a/arch/arm/boot/dts/omap443x.dtsi
+++ b/arch/arm/boot/dts/omap443x.dtsi
@@ -24,4 +24,24 @@
 			clock-latency = <300000>; /* From legacy driver */
 		};
 	};
+
+	abb@0x4a307bd0 {
+		operating-points = <
+			/* kHz   ABB */
+			300000  0
+			600000  0
+			800000  0
+			1008000 1
+		>;
+	};
+
+	abb@0x4a307bd8 {
+		operating-points = <
+			/* kHz   ABB */
+			133000  0
+			266100  0
+			332000  0
+		>;
+	};
+
 };
diff --git a/arch/arm/boot/dts/omap446x.dtsi b/arch/arm/boot/dts/omap446x.dtsi
new file mode 100644
index 0000000..b884c60
--- /dev/null
+++ b/arch/arm/boot/dts/omap446x.dtsi
@@ -0,0 +1,36 @@
+/*
+ * 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@0x4a307bd0 {
+		operating-points = <
+			/* kHz   ABB */
+			350000  0
+			700000  0
+			920000  0
+			1200000 1
+			1500000 1
+		>;
+	};
+
+	abb@0x4a307bd8 {
+		operating-points = <
+			/* kHz   ABB */
+			133000  0
+			266100  0
+			332000  0
+			430000  1
+			500000  1
+		>;
+	};
+
+};
-- 
1.7.9.5

--
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

[PATCH 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.
This data is based on OMAP543x_ES2.0_NDA_TRM_vR.pdf
document.

Cc: Mike Turquette <mturquette@linaro.org>
Cc: Tero Kristo <t-kristo@ti.com>
Cc: Nishanth Menon <nm@ti.com>
Cc: "Benoît Cousson" <b-cousson@ti.com>
Cc: linux-omap@vger.kernel.org
Cc: devicetree-discuss@lists.ozlabs.org

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

diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
index 150bff6..eb11bb7 100644
--- a/arch/arm/boot/dts/omap5.dtsi
+++ b/arch/arm/boot/dts/omap5.dtsi
@@ -74,6 +74,37 @@
 		};
 	};
 
+	abb@0x4ae07cdc {
+		compatible = "ti,omap5-abb";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		reg = <0x4ae07cdc 0x8>,
+		      <0x4ae06014 0x4>; /* PRM_IRQSTATUS_MPU_2 */
+		operating-points = <
+				/* kHz   ABB */
+				499200  0
+				1099800 1
+				1500000 1
+				1699200 1
+		>;
+		ti,tranxdone_status_mask = <0x80>; /* (1 << 7) */
+	};
+
+	abb@0x4ae07ce4 {
+		compatible = "ti,omap5-abb";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		reg = <0x4ae07ce4 0x8>,
+		      <0x4ae06010 0x4>; /* PRM_IRQSTATUS_MPU */
+		operating-points = <
+				/* kHz   ABB */
+				194100	0
+				388310	0
+				531200	0
+		>;
+		ti,tranxdone_status_mask = <0x80000000>; /* (1 << 31) */
+	};
+
 	/*
 	 * XXX: Use a flat representation of the OMAP3 interconnect.
 	 * The real OMAP interconnect network is quite complex.
-- 
1.7.9.5

--
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

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

[Andrii Tseglytskyi]

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

This patch introduces aliases for SYS clock, MPU clock
and IVA clock. These aliases will be used in ABB driver,
which will be introduced in the following patches.

Cc: Mike Turquette <mturquette@linaro.org>
Cc: Tero Kristo <t-kristo@ti.com>
Cc: Nishanth Menon <nm@ti.com>
Cc: "Benoît Cousson" <b-cousson@ti.com>
Cc: linux-omap@vger.kernel.org
Cc: devicetree-discuss@lists.ozlabs.org

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

diff --git a/arch/arm/mach-omap2/cclock3xxx_data.c b/arch/arm/mach-omap2/cclock3xxx_data.c
index 6ef8758..1d0f36d 100644
--- a/arch/arm/mach-omap2/cclock3xxx_data.c
+++ b/arch/arm/mach-omap2/cclock3xxx_data.c
@@ -3494,6 +3494,8 @@ static struct omap_clk omap3xxx_clks[] = {
 	CLK(NULL,	"timer_32k_ck",	&omap_32k_fck,  CK_3XXX),
 	CLK(NULL,	"timer_sys_ck",	&sys_ck,	CK_3XXX),
 	CLK(NULL,	"cpufreq_ck",	&dpll1_ck,	CK_3XXX),
+	CLK("483072f0.abb",	"abb_notify_ck",	&dpll1_ck,	CK_36XX),
+	CLK("483072f0.abb",	"abb_sys_ck",		&sys_ck,	CK_36XX),
 };
 
 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 a2cc046..3eb3a7f 100644
--- a/arch/arm/mach-omap2/cclock44xx_data.c
+++ b/arch/arm/mach-omap2/cclock44xx_data.c
@@ -1978,6 +1978,10 @@ static struct omap_clk omap44xx_clks[] = {
 	CLK("4013c000.timer",	"timer_sys_ck",	&syc_clk_div_ck,	CK_443X),
 	CLK("4013e000.timer",	"timer_sys_ck",	&syc_clk_div_ck,	CK_443X),
 	CLK(NULL,	"cpufreq_ck",	&dpll_mpu_ck,	CK_443X),
+	CLK("4a307bd0.abb",	"abb_notify_ck",	&dpll_mpu_ck,		CK_443X),
+	CLK("4a307bd8.abb",	"abb_notify_ck",	&dpll_iva_m5x2_ck,	CK_443X),
+	CLK("4a307bd0.abb",	"abb_sys_ck",	&sys_clkin_ck,		CK_443X),
+	CLK("4a307bd8.abb",	"abb_sys_ck",	&sys_clkin_ck,		CK_443X),
 };
 
 static const char *enable_init_clks[] = {
diff --git a/arch/arm/mach-omap2/cclock54xx_data.c b/arch/arm/mach-omap2/cclock54xx_data.c
index a9ebed6..b440c9b 100644
--- a/arch/arm/mach-omap2/cclock54xx_data.c
+++ b/arch/arm/mach-omap2/cclock54xx_data.c
@@ -1383,6 +1383,10 @@ static struct omap_clk omap54xx_clks[] = {
 	CLK("4013a000.timer",	"timer_sys_ck",		&dss_syc_gfclk_div, 	CK_54XX),
 	CLK("4013c000.timer",	"timer_sys_ck",		&dss_syc_gfclk_div, 	CK_54XX),
 	CLK("4013e000.timer",	"timer_sys_ck",		&dss_syc_gfclk_div, 	CK_54XX),
+	CLK("4ae07cdc.abb",	"abb_notify_ck",	&dpll_mpu_ck,		CK_54XX),
+	CLK("4ae07ce4.abb",	"abb_notify_ck",        &dpll_iva_h12x2_ck,	CK_54XX),
+	CLK("4ae07cdc.abb",	"abb_sys_ck",		&sys_clkin,		CK_54XX),
+	CLK("4ae07ce4.abb",	"abb_sys_ck",		&sys_clkin,		CK_54XX),
 };
 
 int __init omap5xxx_clk_init(void)
-- 
1.7.9.5

--
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

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

[Andrii Tseglytskyi]

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

This patch introduces the Adaptive Body-Bias LDO driver,
which handles LDOs voltage during OPP change routine.
It follows general principles of ABB implementation
in kernel 3.4.

Some new features are added:

1) ABB driver uses clock notifier framework to scale LDO.
To make this working it handles it's own OPP table.
OPP table has the following format in device tree:

operating-points = <
	       /* kHz   ABB (0 - Bypass, 1 - FBB, 2 - RBB) */
	       499200		0
	       1099800		1
	       1500000		1
	       1699200		1
>;

Generic API is used for OPP table parsing - of_init_opp_table()

2) ABB driver doesn't have any public interfaces. It follows
Voltage/Frequency changes, using only notification mechanism.

3) ABB driver uses PRM_IRQSTATUS register to check tranxdone status.
This register is shared with VP.

Cc: Mike Turquette <mturquette@linaro.org>
Cc: Tero Kristo <t-kristo@ti.com>
Cc: Nishanth Menon <nm@ti.com>
Cc: "Benoît Cousson" <b-cousson@ti.com>
Cc: linux-omap@vger.kernel.org

Signed-off-by: Andrii.Tseglytskyi <andrii.tseglytskyi@ti.com>
Signed-off-by: Mike Turquette <mturquette@linaro.org>
---
 drivers/power/avs/Makefile |    2 +-
 drivers/power/avs/abb.c    |  570 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 571 insertions(+), 1 deletion(-)
 create mode 100644 drivers/power/avs/abb.c

diff --git a/drivers/power/avs/Makefile b/drivers/power/avs/Makefile
index 0843386..d5fc9c4 100644
--- a/drivers/power/avs/Makefile
+++ b/drivers/power/avs/Makefile
@@ -1 +1 @@
-obj-$(CONFIG_POWER_AVS_OMAP)		+= smartreflex.o
+obj-$(CONFIG_POWER_AVS_OMAP)		+= smartreflex.o abb.o
diff --git a/drivers/power/avs/abb.c b/drivers/power/avs/abb.c
new file mode 100644
index 0000000..f5bbb8d
--- /dev/null
+++ b/drivers/power/avs/abb.c
@@ -0,0 +1,570 @@
+/*
+ * 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/clk-provider.h>
+#include <linux/io.h>
+#include <linux/opp.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
+
+/*
+ * 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;
+	struct clk	*notify_clk;
+	struct omap_abb_data	data;
+	struct notifier_block abb_clk_nb;
+};
+
+static const struct omap_abb_data __initdata omap36xx_abb_data = {
+	.opp_sel_mask		= (3 << 0), /* OMAP3630_OPP_SEL_MASK */
+	.opp_change_mask	= (1 << 2), /* OMAP3630_OPP_CHANGE_MASK */
+	.sr2en_mask		= (1 << 0), /* OMAP3630_SR2EN_MASK */
+	.fbb_sel_mask		= (1 << 2), /* OMAP3630_ACTIVE_FBB_SEL_MASK */
+	.sr2_wtcnt_value_mask = (0xff << 8), /* OMAP3630_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		= (0x3 << 0), /* OMAP4430_OPP_SEL_MASK */
+	.opp_change_mask	= (1 << 2), /* OMAP4430_OPP_CHANGE_MASK */
+	.sr2en_mask		= (1 << 0), /* OMAP4430_SR2EN_MASK */
+	.fbb_sel_mask		= (1 << 2), /* OMAP4430_ACTIVE_FBB_SEL_MASK */
+	.rbb_sel_mask		= (1 << 1), /* OMAP4430_ACTIVE_RBB_SEL_MASK */
+	.sr2_wtcnt_value_mask = (0xff << 8), /* OMAP4430_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		= (0x3 << 0), /* OMAP54XX_OPP_SEL_MASK */
+	.opp_change_mask	= (1 << 2), /* OMAP54XX_OPP_CHANGE_MASK */
+	.sr2en_mask		= (1 << 0), /* OMAP54XX_SR2EN_MASK */
+	.fbb_sel_mask		= (1 << 2), /* OMAP54XX_ACTIVE_FBB_SEL_MASK */
+	.rbb_sel_mask		= (1 << 1), /* OMAP54XX_ACTIVE_RBB_SEL_MASK */
+	.sr2_wtcnt_value_mask = (0xff << 8), /* OMAP54XX_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(struct omap_abb *abb, u32 offs)
+{
+	return __raw_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(struct omap_abb *abb, u32 mask, u32 bits, u32 offs)
+{
+	u32 val;
+
+	val = __raw_readl(abb->control + offs);
+	val &= ~mask;
+	val |= bits;
+	__raw_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(struct omap_abb *abb)
+{
+	return !!(__raw_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(struct omap_abb *abb)
+{
+	__raw_writel(abb->txdone_mask, abb->txdone);
+};
+
+/**
+ * omap_abb_wait_tranx() - waits for ABB tranxdone event
+ * @abb:	pointer to the abb instance
+ *
+ * Returns -ETIMEDOUT if the event is not set on time.
+ */
+static int omap_abb_wait_tranx(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 -ETIMEDOUT if the event is not cleared on time.
+ */
+static int omap_abb_clear_tranx(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 based on new voltage
+ * @abb:	pointer to the abb instance
+ * @opp_sel:	target ABB ldo operating mode
+ *
+ * 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, u8 opp_sel)
+{
+	int ret = 0;
+	const struct omap_abb_data *data = &abb->data;
+
+	/* bail early if no transition is necessary */
+	if (opp_sel == abb->opp_sel)
+		return ret;
+
+	/* 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);
+		return -EINVAL;
+	}
+
+	/* 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);
+	WARN_ONCE(ret, "%s: ABB TRANXDONE was not set on time:%d\n",
+		  __func__, ret);
+
+	/* clear interrupt status */
+	ret |= omap_abb_clear_tranx(abb);
+
+out:
+	if (ret) {
+		dev_warn(abb->dev, "%s: failed to scale: opp_sel=%d (%d)\n",
+			 __func__, opp_sel, ret);
+	} else {
+		/* track internal state */
+		abb->opp_sel = opp_sel;
+		dev_dbg(abb->dev, "%s: scaled - opp_sel=%d\n",
+			__func__, opp_sel);
+	}
+	return ret;
+}
+
+/**
+ * omap_abb_pre_scale() - ABB transition pre-frequency scale callback
+ * @abb:	pointer to the ABB instance
+ * @old_rate:	old notifier clock rate
+ * @new_rate:	new notifier clock rate
+ *
+ * Changes the ABB ldo mode prior to scaling the frequency.
+ * Returns 0 on success, otherwise an error code.
+ */
+static int omap_abb_pre_scale(struct omap_abb *abb,
+			      unsigned long old_rate,
+			      unsigned long new_rate)
+{
+	struct opp *opp;
+
+	/* bail if the sequence is wrong */
+	if (new_rate >= old_rate)
+		return 0;
+
+	rcu_read_lock();
+	opp = opp_find_freq_exact(abb->dev, new_rate, true);
+	rcu_read_unlock();
+
+	if (IS_ERR(opp)) {
+		dev_err(abb->dev, "%s: can't find OPP for Freq (%lu)",
+			__func__, new_rate);
+		return -EINVAL;
+	}
+
+	return omap_abb_set_opp(abb, opp_get_voltage(opp));
+}
+
+/**
+ * omap_abb_post_scale() - ABB transition post-frequency scale callback
+ * @abb:	pointer to the ABB instance
+ * @old_rate:	old notifier clock rate
+ * @new_rate:	new notifier clock rate
+ *
+ * Changes the ABB ldo mode prior to scaling the frequency.
+ * Returns 0 on success, otherwise an error code.
+ */
+static int omap_abb_post_scale(struct omap_abb *abb,
+			       unsigned long old_rate,
+			       unsigned long new_rate)
+{
+	struct opp *opp;
+
+	/* bail if the sequence is wrong */
+	if (new_rate <= old_rate)
+		return 0;
+
+	rcu_read_lock();
+	opp = opp_find_freq_exact(abb->dev, new_rate, true);
+	rcu_read_unlock();
+
+	if (IS_ERR(opp)) {
+		dev_err(abb->dev, "%s: can't find OPP for Freq (%lu)",
+			__func__, new_rate);
+		return -EINVAL;
+	}
+
+	return omap_abb_set_opp(abb, opp_get_voltage(opp));
+}
+
+/**
+ * omap_abb_clock_rate_change() - ABB clock notifier callback
+ * @nb:		notifier block
+ * @flags:	notifier event type
+ * @data:	notifier data, contains clock rates
+ *
+ * Returns NOTIFY_OK
+ */
+static int omap_abb_clock_rate_change(struct notifier_block *nb,
+				      unsigned long flags, void *data)
+{
+	struct clk_notifier_data *cnd = data;
+	struct omap_abb *abb = container_of(nb, struct omap_abb, abb_clk_nb);
+
+	switch (flags) {
+	case PRE_RATE_CHANGE:
+		omap_abb_pre_scale(abb, cnd->old_rate, cnd->new_rate);
+		break;
+	case POST_RATE_CHANGE:
+		omap_abb_post_scale(abb, cnd->old_rate, cnd->new_rate);
+		break;
+	}
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block abb_clk_nb = {
+	.notifier_call = omap_abb_clock_rate_change,
+};
+
+#if defined(CONFIG_OF)
+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);
+#endif
+
+/*
+ * 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 clk *sys_clk = NULL;
+	u32 sys_clk_rate, sr2_wt_cnt_val, clock_cycles, abb_sel;
+	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;
+
+	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;
+	}
+
+	/*
+	 * 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.
+	 */
+	ret = of_property_read_u32(pdev->dev.of_node,
+				   "ti,tranxdone_status_mask",
+				   &abb->txdone_mask);
+	if (ret)
+		goto err;
+
+	ret = of_init_opp_table(&pdev->dev);
+	if (ret)
+		goto err;
+
+	abb->notify_clk = clk_get(&pdev->dev, "abb_notify_ck");
+	if (IS_ERR_OR_NULL(abb->notify_clk)) {
+		ret = -ENODEV;
+		goto err;
+	}
+
+	sys_clk = clk_get(&pdev->dev, "abb_sys_ck");
+	if (IS_ERR_OR_NULL(sys_clk)) {
+		ret = -ENODEV;
+		goto err_sys_ck;
+	}
+
+	/* 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);
+
+	abb->abb_clk_nb = abb_clk_nb;
+	clk_notifier_register(abb->notify_clk, &abb->abb_clk_nb);
+
+	/* 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;
+
+err_sys_ck:
+	clk_put(abb->notify_clk);
+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

--
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: [PATCH 5/6] ARM: OMAP3+: ABB: introduce ABB driver

[Mike Turquette]

Quoting Andrii Tseglytskyi (2013-03-28 10:16:07)
> From: "Andrii.Tseglytskyi" <andrii.tseglytskyi@ti.com>
> 
> This patch introduces the Adaptive Body-Bias LDO driver,
> which handles LDOs voltage during OPP change routine.
> It follows general principles of ABB implementation
> in kernel 3.4.
> 
> Some new features are added:
> 
> 1) ABB driver uses clock notifier framework to scale LDO.
> To make this working it handles it's own OPP table.
> OPP table has the following format in device tree:
> 
> operating-points = <
>                /* kHz   ABB (0 - Bypass, 1 - FBB, 2 - RBB) */
>                499200           0
>                1099800          1
>                1500000          1
>                1699200          1
> >;
> 
> Generic API is used for OPP table parsing - of_init_opp_table()
> 
> 2) ABB driver doesn't have any public interfaces. It follows
> Voltage/Frequency changes, using only notification mechanism.
> 
> 3) ABB driver uses PRM_IRQSTATUS register to check tranxdone status.
> This register is shared with VP.

Hi Andrii,

You should probably post this as an RFC.  It remains to be seen if using
the clk rate-change notifiers will be the mechanism for scaling voltage
in The Future.  This patch is helpful for the purposes of that
discussion but shouldn't be merged until there is more consensus.

Some comments below.

<snip>

> +/*
> + * 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

Can you align all of these?

> + */
> +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;
> +       struct clk      *notify_clk;
> +       struct omap_abb_data    data;
> +       struct notifier_block abb_clk_nb;
> +};
> +

Ditto.

> +static const struct omap_abb_data __initdata omap36xx_abb_data = {
> +       .opp_sel_mask           = (3 << 0), /* OMAP3630_OPP_SEL_MASK */
> +       .opp_change_mask        = (1 << 2), /* OMAP3630_OPP_CHANGE_MASK */
> +       .sr2en_mask             = (1 << 0), /* OMAP3630_SR2EN_MASK */
> +       .fbb_sel_mask           = (1 << 2), /* OMAP3630_ACTIVE_FBB_SEL_MASK */
> +       .sr2_wtcnt_value_mask = (0xff << 8), /* OMAP3630_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           = (0x3 << 0), /* OMAP4430_OPP_SEL_MASK */
> +       .opp_change_mask        = (1 << 2), /* OMAP4430_OPP_CHANGE_MASK */
> +       .sr2en_mask             = (1 << 0), /* OMAP4430_SR2EN_MASK */
> +       .fbb_sel_mask           = (1 << 2), /* OMAP4430_ACTIVE_FBB_SEL_MASK */
> +       .rbb_sel_mask           = (1 << 1), /* OMAP4430_ACTIVE_RBB_SEL_MASK */
> +       .sr2_wtcnt_value_mask = (0xff << 8), /* OMAP4430_SR2_WTCNT_VALUE_MASK */
> +       .setup_offs             = 0,
> +       .control_offs           = 0x4,
> +       .settling_time          = 50,
> +       .clock_cycles           = 16,
> +};

Any reason to hard code those values and then put the TRM bitfield name
in a comment?  Probably better to #define these values using those
bitfield names.

Also please be consistent about "0x" in your numeric values.  Mixing it
in randomly (such as 0x3 and 0x4 above) is weird.

> +
> +static const struct omap_abb_data __initdata omap5_abb_data = {
> +       .opp_sel_mask           = (0x3 << 0), /* OMAP54XX_OPP_SEL_MASK */
> +       .opp_change_mask        = (1 << 2), /* OMAP54XX_OPP_CHANGE_MASK */
> +       .sr2en_mask             = (1 << 0), /* OMAP54XX_SR2EN_MASK */
> +       .fbb_sel_mask           = (1 << 2), /* OMAP54XX_ACTIVE_FBB_SEL_MASK */
> +       .rbb_sel_mask           = (1 << 1), /* OMAP54XX_ACTIVE_RBB_SEL_MASK */
> +       .sr2_wtcnt_value_mask = (0xff << 8), /* OMAP54XX_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(struct omap_abb *abb, u32 offs)
> +{
> +       return __raw_readl(abb->control + offs);
> +}

readl instead of __raw_readl?

> +
> +/**
> + * 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(struct omap_abb *abb, u32 mask, u32 bits, u32 offs)
> +{
> +       u32 val;
> +
> +       val = __raw_readl(abb->control + offs);
> +       val &= ~mask;
> +       val |= bits;
> +       __raw_writel(val, abb->control + offs);

writel instead of __raw_writel?

<snip>

> +/**
> + * omap_abb_pre_scale() - ABB transition pre-frequency scale callback
> + * @abb:       pointer to the ABB instance
> + * @old_rate:  old notifier clock rate
> + * @new_rate:  new notifier clock rate
> + *
> + * Changes the ABB ldo mode prior to scaling the frequency.
> + * Returns 0 on success, otherwise an error code.
> + */
> +static int omap_abb_pre_scale(struct omap_abb *abb,
> +                             unsigned long old_rate,
> +                             unsigned long new_rate)
> +{
> +       struct opp *opp;
> +
> +       /* bail if the sequence is wrong */
> +       if (new_rate >= old_rate)
> +               return 0;
> +
> +       rcu_read_lock();
> +       opp = opp_find_freq_exact(abb->dev, new_rate, true);
> +       rcu_read_unlock();
> +
> +       if (IS_ERR(opp)) {
> +               dev_err(abb->dev, "%s: can't find OPP for Freq (%lu)",
> +                       __func__, new_rate);
> +               return -EINVAL;
> +       }
> +
> +       return omap_abb_set_opp(abb, opp_get_voltage(opp));
> +}
> +
> +/**
> + * omap_abb_post_scale() - ABB transition post-frequency scale callback
> + * @abb:       pointer to the ABB instance
> + * @old_rate:  old notifier clock rate
> + * @new_rate:  new notifier clock rate
> + *
> + * Changes the ABB ldo mode prior to scaling the frequency.
> + * Returns 0 on success, otherwise an error code.
> + */
> +static int omap_abb_post_scale(struct omap_abb *abb,
> +                              unsigned long old_rate,
> +                              unsigned long new_rate)
> +{
> +       struct opp *opp;
> +
> +       /* bail if the sequence is wrong */
> +       if (new_rate <= old_rate)
> +               return 0;
> +
> +       rcu_read_lock();
> +       opp = opp_find_freq_exact(abb->dev, new_rate, true);
> +       rcu_read_unlock();
> +
> +       if (IS_ERR(opp)) {
> +               dev_err(abb->dev, "%s: can't find OPP for Freq (%lu)",
> +                       __func__, new_rate);
> +               return -EINVAL;
> +       }
> +
> +       return omap_abb_set_opp(abb, opp_get_voltage(opp));
> +}
> +
> +/**
> + * omap_abb_clock_rate_change() - ABB clock notifier callback
> + * @nb:                notifier block
> + * @flags:     notifier event type
> + * @data:      notifier data, contains clock rates
> + *
> + * Returns NOTIFY_OK
> + */
> +static int omap_abb_clock_rate_change(struct notifier_block *nb,
> +                                     unsigned long flags, void *data)
> +{
> +       struct clk_notifier_data *cnd = data;
> +       struct omap_abb *abb = container_of(nb, struct omap_abb, abb_clk_nb);
> +
> +       switch (flags) {
> +       case PRE_RATE_CHANGE:
> +               omap_abb_pre_scale(abb, cnd->old_rate, cnd->new_rate);
> +               break;
> +       case POST_RATE_CHANGE:
> +               omap_abb_post_scale(abb, cnd->old_rate, cnd->new_rate);
> +               break;
> +       }
> +
> +       return NOTIFY_OK;
> +}

How does this synchronize with the VC/VP voltage transition?  Ordering
is important here and if the clk rate-change notifiers are used for both
a VP force update and for an FBB transition there is no guarantee of the
ordering.

Regards,
Mike

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

[Nishanth Menon]

- ti internal list which got into the git send-email by error. Apologies
on the bounces which might have resulted...

On 14:27-20130328, Mike Turquette wrote:
> Quoting Andrii Tseglytskyi (2013-03-28 10:16:07)
> > From: "Andrii.Tseglytskyi" <andrii.tseglytskyi@ti.com>
Ownership of ABB driver patch probably still belongs to Mike as the original
author of the code and working out all the details to entitle ABB on OMAP
platforms :)
> > 
[..]
> You should probably post this as an RFC.  It remains to be seen if using
> the clk rate-change notifiers will be the mechanism for scaling voltage
> in The Future.  This patch is helpful for the purposes of that
> discussion but shouldn't be merged until there is more consensus.
I agree. + include linux-arm-kernel and lkml in CC on the next *public*
revision :).

[..]
> 
> > +/*
> > + * 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
> 
> Can you align all of these?
We should check up on AM and DM series as well - if the bit offsets are
precisely the same, we could stick with macros instead of having the
masks and register offsets in a device specific manner.

[..]
> > +/**
> > + * 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(struct omap_abb *abb, u32 offs)
> > +{
> > +       return __raw_readl(abb->control + offs);
> > +}
> 
> readl instead of __raw_readl?
Ack. readl/writel should be used..

> > +/**
> > + * omap_abb_clock_rate_change() - ABB clock notifier callback
> > + * @nb:                notifier block
> > + * @flags:     notifier event type
> > + * @data:      notifier data, contains clock rates
> > + *
> > + * Returns NOTIFY_OK
> > + */
> > +static int omap_abb_clock_rate_change(struct notifier_block *nb,
> > +                                     unsigned long flags, void *data)
> > +{
> > +       struct clk_notifier_data *cnd = data;
> > +       struct omap_abb *abb = container_of(nb, struct omap_abb, abb_clk_nb);
> > +
> > +       switch (flags) {
> > +       case PRE_RATE_CHANGE:
> > +               omap_abb_pre_scale(abb, cnd->old_rate, cnd->new_rate);
> > +               break;
> > +       case POST_RATE_CHANGE:
> > +               omap_abb_post_scale(abb, cnd->old_rate, cnd->new_rate);
> > +               break;
> > +       }
> > +
> > +       return NOTIFY_OK;
> > +}
> 
> How does this synchronize with the VC/VP voltage transition?  Ordering
> is important here and if the clk rate-change notifiers are used for both
> a VP force update and for an FBB transition there is no guarantee of the
> ordering.
I need to dig into this series deeper, but the requirement is as
follows:
Actual voltage change may occur with vc/vp - OMAP3,4,5 OR with i2c1- AM
series or the upcoming DRA SoCs, intent is to model the vc-vp->pmic as
regulators (some internal patches circulate for this, but not mature enough
to be send out in a public list yet)

Key however is:
* if we do as clock notifiers(as this series attempts I believe), we'd get ABB
settings around clock change boundary.
Alternative might be to do it around voltage change - we could, in
theory:
a) make an omap voltage regulator and provide notifier around the same
and hook ABB to it. the omap voltage regulator will in turn call the
appropriate voltage regulator (modelled from a regulator that controls
an PMIC over i2c1 OR over vc-vp regulator)
b) make ABB as an regulator by itself?
c) how might this work if we have DVFS capability in CCF itself[1]? it
might be better to have it as notifiers to regulator (the pseudo
omap-voltage regulator perhaps?)

[1] CCF DVFS patches:
https://patchwork.kernel.org/patch/2195431/
https://patchwork.kernel.org/patch/2195421/
https://patchwork.kernel.org/patch/2195451/
https://patchwork.kernel.org/patch/2195441/
https://patchwork.kernel.org/patch/2195461/

-- 
Regards,
Nishanth Menon

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

[Andrii Tseglytskyi]

Hi,

Subject changed to "Re: [*RFC* PATCH 5/6] ARM: OMAP3+: ABB: introduce 
ABB driver"

On 03/29/2013 12:35 AM, Nishanth Menon wrote:
>
> - ti internal list which got into the git send-email by error. 
> Apologies on the bounces which might have resulted... On 
> 14:27-20130328, Mike Turquette wrote:
>>
>> Quoting Andrii Tseglytskyi (2013-03-28 10:16:07)
>>> From: "Andrii.Tseglytskyi"<andrii.tseglytskyi@ti.com>
> Ownership of ABB driver patch probably still belongs to Mike as the 
> original
> author of the code and working out all the details to entitle ABB on OMAP
> platforms

Agreed with Mike to keep mine ownership of patch series, and mention 
that Mike is an author of idea and architecture of ABB driver.

> [..]
>> You should probably post this as an RFC.  It remains to be seen if using
>> the clk rate-change notifiers will be the mechanism for scaling voltage
>> in The Future.  This patch is helpful for the purposes of that
>> discussion but shouldn't be merged until there is more consensus.
> I agree. + include linux-arm-kernel and lkml in CC on the next *public*
> revision .

Agree.

>
> [..]
>>> +/*
>>> + * 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
>> Can you align all of these?
> We should check up on AM and DM series as well - if the bit offsets are
> precisely the same, we could stick with macros instead of having the
> masks and register offsets in a device specific manner.

These offsets are the same for OMAP3/4/5. I see two options here.
1) Use macro to define offsets and continue storing them in "struct 
omap_abb_data"
2) Use macro definitions directly in the driver code and remove 
mentioned fields from "struct omap_abb_data"

> [..]
>>> +/**
>>> + * 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(struct omap_abb *abb, u32 offs)
>>> +{
>>> +       return __raw_readl(abb->control + offs);
>>> +}
>> readl instead of __raw_readl?
> Ack. readl/writel should be used..

Ack. readl/writel to be used.

>>> +/**
>>> + * omap_abb_clock_rate_change() - ABB clock notifier callback
>>> + * @nb:                notifier block
>>> + * @flags:     notifier event type
>>> + * @data:      notifier data, contains clock rates
>>> + *
>>> + * Returns NOTIFY_OK
>>> + */
>>> +static int omap_abb_clock_rate_change(struct notifier_block *nb,
>>> +                                     unsigned long flags, void *data)
>>> +{
>>> +       struct clk_notifier_data *cnd = data;
>>> +       struct omap_abb *abb = container_of(nb, struct omap_abb, 
>>> abb_clk_nb);
>>> +
>>> +       switch (flags) {
>>> +       case PRE_RATE_CHANGE:
>>> +               omap_abb_pre_scale(abb, cnd->old_rate, cnd->new_rate);
>>> +               break;
>>> +       case POST_RATE_CHANGE:
>>> +               omap_abb_post_scale(abb, cnd->old_rate, cnd->new_rate);
>>> +               break;
>>> +       }
>>> +
>>> +       return NOTIFY_OK;
>>> +}
>> How does this synchronize with the VC/VP voltage transition? Ordering
>> is important here and if the clk rate-change notifiers are used for both
>> a VP force update and for an FBB transition there is no guarantee of the
>> ordering.
> I need to dig into this series deeper, but the requirement is as
> follows:
> Actual voltage change may occur with vc/vp - OMAP3,4,5 OR with i2c1- AM
> series or the upcoming DRA SoCs, intent is to model the vc-vp->pmic as
> regulators (some internal patches circulate for this, but not mature 
> enough
> to be send out in a public list yet)
>
> Key however is:
> * if we do as clock notifiers(as this series attempts I believe), we'd 
> get ABB
> settings around clock change boundary.
> Alternative might be to do it around voltage change - we could, in
> theory:
> a) make an omap voltage regulator and provide notifier around the same
> and hook ABB to it. the omap voltage regulator will in turn call the
> appropriate voltage regulator (modelled from a regulator that controls
> an PMIC over i2c1 OR over vc-vp regulator)
> b) make ABB as an regulator by itself?
> c) how might this work if we have DVFS capability in CCF itself[1]? it
> might be better to have it as notifiers to regulator (the pseudo
> omap-voltage regulator perhaps?)
>
> [1] CCF DVFS patches:
> https://patchwork.kernel.org/patch/2195431/
> https://patchwork.kernel.org/patch/2195421/
> https://patchwork.kernel.org/patch/2195451/
> https://patchwork.kernel.org/patch/2195441/
> https://patchwork.kernel.org/patch/2195461/
>

In case if VC/VP use clock notifier to scale voltage, there is no 
guarantee of order.
The only option which I see is to create ABB API and call it from OMAP 
regulator during OPP change.

omap_abb_pre_scale(struct omap_abb *abb, u32 old_volt, u32 new_volt);
omap_abb_post_scale(struct omap_abb *abb, u32 old_volt, u32 new_volt);

Mike, do you agree to proceed in this way? Also I need you opinion about 
files placement. Now it is placed in

*drivers/power/avs/abb.c*

And header will be added to
*include/linux/power/abb.h*

Regards,
Andrii

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

[Andrii Tseglytskyi]

Hi,

Subject changed to "Re: [*RFC* PATCH 5/6] ARM: OMAP3+: ABB: introduce 
ABB driver"

On 03/29/2013 12:35 AM, Nishanth Menon wrote:
>
> - ti internal list which got into the git send-email by error. 
> Apologies on the bounces which might have resulted... On 
> 14:27-20130328, Mike Turquette wrote:
>>
>> Quoting Andrii Tseglytskyi (2013-03-28 10:16:07)
>>> From: "Andrii.Tseglytskyi"<andrii.tseglytskyi@ti.com>
> Ownership of ABB driver patch probably still belongs to Mike as the 
> original
> author of the code and working out all the details to entitle ABB on OMAP
> platforms

Agreed with Mike to keep mine ownership of patch series, and mention 
that Mike is an author of idea and architecture of ABB driver.

> [..]
>> You should probably post this as an RFC.  It remains to be seen if using
>> the clk rate-change notifiers will be the mechanism for scaling voltage
>> in The Future.  This patch is helpful for the purposes of that
>> discussion but shouldn't be merged until there is more consensus.
> I agree. + include linux-arm-kernel and lkml in CC on the next *public*
> revision .

Agree.

>
> [..]
>>> +/*
>>> + * 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
>> Can you align all of these?
> We should check up on AM and DM series as well - if the bit offsets are
> precisely the same, we could stick with macros instead of having the
> masks and register offsets in a device specific manner.

These offsets are the same for OMAP3/4/5. I see two options here.
1) Use macro to define offsets and continue storing them in "struct 
omap_abb_data"
2) Use macro definitions directly in the driver code and remove 
mentioned fields from "struct omap_abb_data"

> [..]
>>> +/**
>>> + * 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(struct omap_abb *abb, u32 offs)
>>> +{
>>> +       return __raw_readl(abb->control + offs);
>>> +}
>> readl instead of __raw_readl?
> Ack. readl/writel should be used..

Ack. readl/writel to be used.

>>> +/**
>>> + * omap_abb_clock_rate_change() - ABB clock notifier callback
>>> + * @nb:                notifier block
>>> + * @flags:     notifier event type
>>> + * @data:      notifier data, contains clock rates
>>> + *
>>> + * Returns NOTIFY_OK
>>> + */
>>> +static int omap_abb_clock_rate_change(struct notifier_block *nb,
>>> +                                     unsigned long flags, void *data)
>>> +{
>>> +       struct clk_notifier_data *cnd = data;
>>> +       struct omap_abb *abb = container_of(nb, struct omap_abb, 
>>> abb_clk_nb);
>>> +
>>> +       switch (flags) {
>>> +       case PRE_RATE_CHANGE:
>>> +               omap_abb_pre_scale(abb, cnd->old_rate, cnd->new_rate);
>>> +               break;
>>> +       case POST_RATE_CHANGE:
>>> +               omap_abb_post_scale(abb, cnd->old_rate, cnd->new_rate);
>>> +               break;
>>> +       }
>>> +
>>> +       return NOTIFY_OK;
>>> +}
>> How does this synchronize with the VC/VP voltage transition? Ordering
>> is important here and if the clk rate-change notifiers are used for both
>> a VP force update and for an FBB transition there is no guarantee of the
>> ordering.
> I need to dig into this series deeper, but the requirement is as
> follows:
> Actual voltage change may occur with vc/vp - OMAP3,4,5 OR with i2c1- AM
> series or the upcoming DRA SoCs, intent is to model the vc-vp->pmic as
> regulators (some internal patches circulate for this, but not mature 
> enough
> to be send out in a public list yet)
>
> Key however is:
> * if we do as clock notifiers(as this series attempts I believe), we'd 
> get ABB
> settings around clock change boundary.
> Alternative might be to do it around voltage change - we could, in
> theory:
> a) make an omap voltage regulator and provide notifier around the same
> and hook ABB to it. the omap voltage regulator will in turn call the
> appropriate voltage regulator (modelled from a regulator that controls
> an PMIC over i2c1 OR over vc-vp regulator)
> b) make ABB as an regulator by itself?
> c) how might this work if we have DVFS capability in CCF itself[1]? it
> might be better to have it as notifiers to regulator (the pseudo
> omap-voltage regulator perhaps?)
>
> [1] CCF DVFS patches:
> https://patchwork.kernel.org/patch/2195431/
> https://patchwork.kernel.org/patch/2195421/
> https://patchwork.kernel.org/patch/2195451/
> https://patchwork.kernel.org/patch/2195441/
> https://patchwork.kernel.org/patch/2195461/
>

In case if VC/VP use clock notifier to scale voltage, there is no 
guarantee of order.
The only option which I see is to create ABB API and call it from OMAP 
regulator during OPP change.

omap_abb_pre_scale(struct omap_abb *abb, u32 old_volt, u32 new_volt);
omap_abb_post_scale(struct omap_abb *abb, u32 old_volt, u32 new_volt);

Mike, do you agree to proceed in this way? Also I need you opinion about 
files placement. Now it is placed in

*drivers/power/avs/abb.c*

And header will be added to
*include/linux/power/abb.h*

Regards,
Andrii

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

[Mike Turquette]

Andrii,

Sorry to nitpick further but this your replies look very non-standard.
Typically a right chevron and a space is used to indent replies instead
of a tab.  Something like this:
https://wiki.openstack.org/wiki/MailingListEtiquette#Reply_Level_Indication

Quoting Andrii Tseglytskyi (2013-04-01 03:53:25)
> In case if VC/VP use clock notifier to scale voltage, there is no guarantee of
> order.
> The only option which I see is to create ABB API and call it from OMAP
> regulator during OPP change.
> 

I doubt that is the only option.  Do you mean it is the only option to
quickly get it working right now?

The VC & VP code should be converted to the regulator framework if not
already.  After that is done there are some options for how ABB is
handled.

The VC & VP regulator driver could directly call the api's you list
below in their .set_voltage callback.

Additionally if the regulator is reentrant then ABB could be modeled as
a regulator itself and the VC or VP .set_voltage callback could perhaps
call regulator_set_mode(abb_reg, FBB_MODE).

Creating a regulator for each ABB instance may be overkill or may not be
overkill... that IP has been around since 3630 so several chips use it.

> omap_abb_pre_scale(struct omap_abb *abb, u32 old_volt, u32 new_volt);
> omap_abb_post_scale(struct omap_abb *abb, u32 old_volt, u32 new_volt);
> 
> Mike, do you agree to proceed in this way? Also I need you opinion about files
> placement. Now it is placed in
> 

The above code looks like a quick solution to me.  The long-term
upstream path for this code needs be decided first.  If everything is
going to get converted to the regulator framework then I do not agree to
proceed that way.

Let's figure out what is happening to the VC/VP code first and then
figure out what to do about ABB.

Regards,
Mike

> drivers/power/avs/abb.c
> 
> And header will be added to
> include/linux/power/abb.h
> 
> Regards,
> Andrii

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

[Nishanth Menon]

On Mon, Apr 1, 2013 at 1:10 PM, Mike Turquette <mturquette@linaro.org> wrote:
> Andrii,
>
> Sorry to nitpick further but this your replies look very non-standard.
> Typically a right chevron and a space is used to indent replies instead
> of a tab.  Something like this:
> https://wiki.openstack.org/wiki/MailingListEtiquette#Reply_Level_Indication
>
> Quoting Andrii Tseglytskyi (2013-04-01 03:53:25)
>> In case if VC/VP use clock notifier to scale voltage, there is no guarantee of
>> order.
>> The only option which I see is to create ABB API and call it from OMAP
>> regulator during OPP change.
>>
>
> I doubt that is the only option.  Do you mean it is the only option to
> quickly get it working right now?
>
> The VC & VP code should be converted to the regulator framework if not
> already.  After that is done there are some options for how ABB is
> handled.
>
> The VC & VP regulator driver could directly call the api's you list
> below in their .set_voltage callback.
>
> Additionally if the regulator is reentrant then ABB could be modeled as
> a regulator itself and the VC or VP .set_voltage callback could perhaps
> call regulator_set_mode(abb_reg, FBB_MODE).
>
> Creating a regulator for each ABB instance may be overkill or may not be
> overkill... that IP has been around since 3630 so several chips use it.
>
>> omap_abb_pre_scale(struct omap_abb *abb, u32 old_volt, u32 new_volt);
>> omap_abb_post_scale(struct omap_abb *abb, u32 old_volt, u32 new_volt);
>>
>> Mike, do you agree to proceed in this way? Also I need you opinion about files
>> placement. Now it is placed in
>>
>
> The above code looks like a quick solution to me.  The long-term
> upstream path for this code needs be decided first.  If everything is
> going to get converted to the regulator framework then I do not agree to
> proceed that way.
>
> Let's figure out what is happening to the VC/VP code first and then
> figure out what to do about ABB.
>
http://picpaste.com/KmqDYTn0.jpg
is an quick depiction of the thought I have in my mind.

We do have, in the upcoming SoCs, where Nominal Voltages per device,
will now be encoded into the efuse itself(so called class 0 voltage).
Future SoCs will need to be able to use ABB along with standard
regulators (without use of VC-VP) - in fact, even today, SoCs like AM
and DM series of processors have the same requirement.

We also will have to support class 2 variants of AVS (which will use
standard regulators to set voltage).

As of date, CCF does not control regulators - which means the interim
solution would be for the device control to manipulate both clock and
regulator voltage (similar to what cpufreq-cpu0 driver does today).
these drivers should not know the existance of SoC specific
intricacies - so ABB linked to voltage values make more sense if ABB
sequencing is handled in "TI regulator"

Intent of VC/VP regulator is to be replaceable, on required platforms,
with appropriate regulator which do not use VC/VP paths (e.g. on SoCs
that do not have it).

There is also need to support multiple voltage rails supplied by a
single SMPS - in which case controlling via regulator framework is
more desirable.

---
Regards,
NIshanth Menon

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

[Nishanth Menon]

On 14:34-20130401, Mike Turquette wrote:
> Quoting Nishanth Menon (2013-04-01 12:28:20)
> > On Mon, Apr 1, 2013 at 1:10 PM, Mike Turquette <mturquette@linaro.org> wrote:
> > > Let's figure out what is happening to the VC/VP code first and then
> > > figure out what to do about ABB.
> > >
> > http://picpaste.com/KmqDYTn0.jpg
> > is an quick depiction of the thought I have in my mind.
> > 
> 
> OK, looks like the Super Regulator Approach(tm).
:D I hope it can be modelled after TI SoCs and then expanded to other
SoCs as needed.
> 
> > We do have, in the upcoming SoCs, where Nominal Voltages per device,
> > will now be encoded into the efuse itself(so called class 0 voltage).
> > Future SoCs will need to be able to use ABB along with standard
> > regulators (without use of VC-VP) - in fact, even today, SoCs like AM
> > and DM series of processors have the same requirement.
> > 
> 
> This de-linking of ABB from VC/VP probably supports modeling ABB ldo's
> as Linux regulators.  That would provide a common interface where either
> the VC/VP code or another regulator could call something like
> regulator_set_mode.
> 
> If we're going to export something which can get called by different
> actors it's best to use an already exported interface which the
> regulator framework supplies, instead of exporting something new and
> omap-specific.
Yep - that was my original thought - though modeling it as a regulator
in itself is, IMHO, I think is a good idea.

> 
> > We also will have to support class 2 variants of AVS (which will use
> > standard regulators to set voltage).
> > 
> > As of date, CCF does not control regulators - which means the interim
> > solution would be for the device control to manipulate both clock and
> > regulator voltage (similar to what cpufreq-cpu0 driver does today).
> > these drivers should not know the existance of SoC specific
> > intricacies - so ABB linked to voltage values make more sense if ABB
> > sequencing is handled in "TI regulator"
> > 
> > Intent of VC/VP regulator is to be replaceable, on required platforms,
> > with appropriate regulator which do not use VC/VP paths (e.g. on SoCs
> > that do not have it).
> > 
> 
> Well if everything is nicely modeled then I suppose per-board and
> per-soc DT will give you the ability to link things up nicely.  E.g.
> replacing VC/VP with an external physical LDO, etc.

True - that is precisely what are attempting to do in (hopefully) edible
stages :).

-- 
Regards,
Nishanth Menon

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

[Nishanth Menon]

On 17:05-20130401, Mike Turquette wrote:
> OK, so we're in agreement on what The Future looks like.  What does that
> mean for Andrii's patchset?
Unless anyone has an fundamental issue with the approach of an "Super
regulator" controlling "sub regulators", I think, in-line with your
view, we should probably make ABB as an regulator instead of inventing
our own API and hooking it around clock notifiers.
-- 
Regards,
Nishanth Menon

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

[Grygorii Strashko]

On 04/02/2013 06:35 AM, Nishanth Menon wrote:
> On 17:05-20130401, Mike Turquette wrote:
>> OK, so we're in agreement on what The Future looks like.  What does that
>> mean for Andrii's patchset?
> Unless anyone has an fundamental issue with the approach of an "Super
> regulator" controlling "sub regulators", I think, in-line with your
> view, we should probably make ABB as an regulator instead of inventing
> our own API and hooking it around clock notifiers.
Hi Nishanth, All
One question here, regarding "Super regulator" - How are you going to 
differentiate
OPP changing and AVS Voltage adjustment requests to the "Super regulator"??
As you know, to select OPP changing direction, ABB type (or VC/VP 
parameters)
properly you need Nominal (and only Nominal) voltage as input.
And in real world, AVS can adjust voltage, as example, for OPP100 even 
low than for OPP50.
OMAP4 example:
     MPU OPP    Vsr                Vnom   ABB
     OPP50      0.862249970436096  1.025  NOM
     OPP100     1.03509700298309   1.2    NOM
     OPPTurbo   1.09257805347443   1.325  NOM
     OPPNitro   1.18703103065491   1.388  FAST
     OPPNitroSB 1.29427194595337   1.398  FAST
So, while adjusting voltage, AVS can hit other OPP voltage and, as 
result, ABB mode may be changed.

I think, your vision would be more clear if you could be able to provide 
Sequence diagram in addition.

And would it be allowed to use DT for such regulator chain definitions
(or board-generic.c should be used instead), just for clarification,
  because I have not to much DT experience:
omap443x.dtsi:
vdd_mpu: regulator-omap-ti1 { << -- Super regulator
          compatible = "ti,omapX-regulator";
          regulator-min-microvolt = <750000>;
          regulator-max-microvolt = <1388000>;
          smps-supply = <&smps_mpu>; [or smps-supply = <&vcc>;]
          abb-supply = <&abb_mpu>;
};

smps_mpu: regulator-omap-smps1 { <--VC/VP regulator
          compatible = "ti,omapX-smps-regulator";
          ti,vc = <&vc>
          ti,vp = <&vp_mpu>
};

twl4030.dtsi
vcc: regulator-vdd1 {
           compatible = "ti,twl4030-vdd1";
           regulator-min-microvolt = <600000>;
           regulator-max-microvolt = <1450000>;
};

Regards,
Grygorii Strashko

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

[Nishanth Menon]

On 13:15-20130402, Grygorii Strashko wrote:
> On 04/02/2013 06:35 AM, Nishanth Menon wrote:
> >On 17:05-20130401, Mike Turquette wrote:
> >>OK, so we're in agreement on what The Future looks like.  What does that
> >>mean for Andrii's patchset?
> >Unless anyone has an fundamental issue with the approach of an "Super
> >regulator" controlling "sub regulators", I think, in-line with your
> >view, we should probably make ABB as an regulator instead of inventing
> >our own API and hooking it around clock notifiers.
> Hi Nishanth, All
> One question here, regarding "Super regulator" - How are you going
> to differentiate
> OPP changing and AVS Voltage adjustment requests to the "Super regulator"??
> As you know, to select OPP changing direction, ABB type (or VC/VP
> parameters)
> properly you need Nominal (and only Nominal) voltage as input.
> And in real world, AVS can adjust voltage, as example, for OPP100
> even low than for OPP50.
> OMAP4 example:
>     MPU OPP    Vsr                Vnom   ABB
>     OPP50      0.862249970436096  1.025  NOM
>     OPP100     1.03509700298309   1.2    NOM
>     OPPTurbo   1.09257805347443   1.325  NOM
>     OPPNitro   1.18703103065491   1.388  FAST
>     OPPNitroSB 1.29427194595337   1.398  FAST
> So, while adjusting voltage, AVS can hit other OPP voltage and, as
> result, ABB mode may be changed.
The expectation is that we will use Vnom in the "super regulator". Vnom
does not change per OPP across devices. Expectation will be to use Vnom
as indexing key inside regulator framework.

> 
> I think, your vision would be more clear if you could be able to
> provide Sequence diagram in addition.
I should be able to do better, I should be able to post a RFC revision
of the "super regulator" verifiable on Beagle XM using I2C1 (instead of
i2c_sr) as start, I will give it hooks to handle "Efuse Vnom" support
which our upcoming SoC needs as well.
> 
> And would it be allowed to use DT for such regulator chain definitions
> (or board-generic.c should be used instead), just for clarification,
>  because I have not to much DT experience:
> omap443x.dtsi:
> vdd_mpu: regulator-omap-ti1 { << -- Super regulator
I would not link only *OMAP* to this - as originally stated, this is
needed by OMAP, AM, DM and potentially, Keystone family of TI
processors.

How about we debate this with the "super regulator" patch?

[...]
-- 
Regards,
Nishanth Menon

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

[Andrii Tseglytskyi]

On 04/02/2013 08:16 PM, Mike Turquette wrote:
> Quoting Nishanth Menon (2013-04-01 20:35:45)
>> On 17:05-20130401, Mike Turquette wrote:
>>> OK, so we're in agreement on what The Future looks like.  What does that
>>> mean for Andrii's patchset?
>> Unless anyone has an fundamental issue with the approach of an "Super
>> regulator" controlling "sub regulators", I think, in-line with your
>> view, we should probably make ABB as an regulator instead of inventing
>> our own API and hooking it around clock notifiers.
> ACK.  Making the ABB code into a regulator driver is the right thing to
> do regardless of whether or not we use a Super Regulator(tm) or just
> chain together Not So Super Regulators(tm).
>
> I'm not an expert at the regulator framework, but I encourage Andrii to
> look into regulator_set_mode(), which might be a more semantically
> accurate alternative than regulator_set_voltage() for the ABB ldo.
>
> Regards,
> Mike
>
>> -- 
>> Regards,
>> Nishanth Menon

Hi,

Agree. It is a good idea in general.
regulator_set_mode() API seems to be good enough for handling ABB mode 
(FBB/RBB/Bypass).
Knowledge about ABB mode on each OPP can be moved from ABB regulator to 
"Super regulator".
Thanks a lot for all your comments.

Regards,
Andrii

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

[Nishanth Menon]

On 20:35-20130402, Andrii Tseglytskyi wrote:
> On 04/02/2013 08:16 PM, Mike Turquette wrote:
> >Quoting Nishanth Menon (2013-04-01 20:35:45)
> >>On 17:05-20130401, Mike Turquette wrote:
> >>>OK, so we're in agreement on what The Future looks like.  What does that
> >>>mean for Andrii's patchset?
> >>Unless anyone has an fundamental issue with the approach of an "Super
> >>regulator" controlling "sub regulators", I think, in-line with your
> >>view, we should probably make ABB as an regulator instead of inventing
> >>our own API and hooking it around clock notifiers.
> >ACK.  Making the ABB code into a regulator driver is the right thing to
> >do regardless of whether or not we use a Super Regulator(tm) or just
> >chain together Not So Super Regulators(tm).
> >
> >I'm not an expert at the regulator framework, but I encourage Andrii to
> >look into regulator_set_mode(), which might be a more semantically
> >accurate alternative than regulator_set_voltage() for the ABB ldo.
> 
> 
> Agree. It is a good idea in general.
> regulator_set_mode() API seems to be good enough for handling ABB
> mode (FBB/RBB/Bypass).
> Knowledge about ABB mode on each OPP can be moved from ABB regulator
> to "Super regulator".
> Thanks a lot for all your comments.
> 

Digging a little more on this:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/include/linux/regulator/consumer.h#n41

If we were to mean usage of mode to mean - usage of PWM/PFM etc
mode(like in tps/twl chips), this makes sense. However, if we mean
forward, reverse and bypass as "modes" we might be misusing the original
intent of the API.

-- 
Regards,
Nishanth Menon

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

[Mike Turquette]

Quoting Nishanth Menon (2013-04-02 19:00:02)
> On 20:35-20130402, Andrii Tseglytskyi wrote:
> > On 04/02/2013 08:16 PM, Mike Turquette wrote:
> > >Quoting Nishanth Menon (2013-04-01 20:35:45)
> > >>On 17:05-20130401, Mike Turquette wrote:
> > >>>OK, so we're in agreement on what The Future looks like.  What does that
> > >>>mean for Andrii's patchset?
> > >>Unless anyone has an fundamental issue with the approach of an "Super
> > >>regulator" controlling "sub regulators", I think, in-line with your
> > >>view, we should probably make ABB as an regulator instead of inventing
> > >>our own API and hooking it around clock notifiers.
> > >ACK.  Making the ABB code into a regulator driver is the right thing to
> > >do regardless of whether or not we use a Super Regulator(tm) or just
> > >chain together Not So Super Regulators(tm).
> > >
> > >I'm not an expert at the regulator framework, but I encourage Andrii to
> > >look into regulator_set_mode(), which might be a more semantically
> > >accurate alternative than regulator_set_voltage() for the ABB ldo.
> > 
> > 
> > Agree. It is a good idea in general.
> > regulator_set_mode() API seems to be good enough for handling ABB
> > mode (FBB/RBB/Bypass).
> > Knowledge about ABB mode on each OPP can be moved from ABB regulator
> > to "Super regulator".
> > Thanks a lot for all your comments.
> > 
> 
> Digging a little more on this:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/include/linux/regulator/consumer.h#n41
> 
> If we were to mean usage of mode to mean - usage of PWM/PFM etc
> mode(like in tps/twl chips), this makes sense. However, if we mean
> forward, reverse and bypass as "modes" we might be misusing the original
> intent of the API.

Yeah, I agree that using those modes would probably qualify as abuse.
However I still find it tempting to use FAST for FBB and maybe NORMAL or
STANDBY for bypass.

Instead of using a mode then a voltage could be used.  Any Vnom value
passed into regulator_set_voltage would result in ABB ldo being
bypassed, whereas if 900mV was passed in that would put the ABB ldo into
FBB.  And that 900mV value isn't really set in stone, it is just more
often than not the value observed on OMAP3630 and OMAP4.

However that is really a kludge and completely non-intuitive for someone
looking at the code for the first time.  I haven't gone digging through
the regulator stuff concerning this but I hope a good solution can be
found.

Regards,
Mike

> 
> -- 
> Regards,
> Nishanth Menon

[PATCH 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.

Cc: Mike Turquette <mturquette@linaro.org>
Cc: Tero Kristo <t-kristo@ti.com>
Cc: Nishanth Menon <nm@ti.com>
Cc: "Benoît Cousson" <b-cousson@ti.com>
Cc: linux-omap@vger.kernel.org

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

diff --git a/drivers/power/avs/abb.c b/drivers/power/avs/abb.c
index f5bbb8d..ac43841 100644
--- a/drivers/power/avs/abb.c
+++ b/drivers/power/avs/abb.c
@@ -19,6 +19,7 @@
 #include <linux/clk-provider.h>
 #include <linux/io.h>
 #include <linux/opp.h>
+#include <linux/debugfs.h>
 
 /* NOMINAL_OPP bypasses the ABB ldo, FAST_OPP sets it to Forward Body-Bias */
 #define OMAP_ABB_NOMINAL_OPP	0
@@ -410,6 +411,66 @@ static const struct of_device_id __initdata omap_abb_of_match[] = {
 MODULE_DEVICE_TABLE(of, omap_abb_of_match);
 #endif
 
+static int omap_abb_info_dump(struct seq_file *sf, void *unused)
+{
+	struct opp *opp;
+	struct omap_abb *abb = (struct omap_abb *)sf->private;
+	u32 abb_ctrl, abb_setup, rate;
+	unsigned long freq = 0;
+
+	if (!abb) {
+		seq_printf(sf, "No ABB defined\n");
+		goto out;
+	}
+
+	abb_ctrl =  omap_abb_readl(abb, abb->data.control_offs);
+	abb_setup = omap_abb_readl(abb, abb->data.setup_offs);
+	rate = clk_get_rate(abb->notify_clk);
+	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"
+		   "Notify clk\t->\t(%s) rate (%u)\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,
+		   __clk_get_name(abb->notify_clk), rate);
+
+	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, "Freq (%lu) ABB (%lu)\n", opp_get_freq(opp),
+			   opp_get_voltage(opp));
+
+		freq++;
+	} while (1);
+	rcu_read_unlock();
+
+out:
+	return 0;
+}
+
+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_probe() - Initialize an ABB ldo instance
  * @pdev: ABB platform device
@@ -544,6 +605,9 @@ static int __init omap_abb_probe(struct platform_device *pdev)
 		omap_abb_rmw(abb, abb->data.sr2en_mask,
 			     abb->data.sr2en_mask, abb->data.setup_offs);
 
+	debugfs_create_file(dev_name(abb->dev), S_IRUGO, NULL,
+			    abb, &omap_abb_debug_fops);
+
 	clk_put(sys_clk);
 	return 0;
 
-- 
1.7.9.5

--
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