linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V3 00/10] PM: Create the AVS(Adaptive Voltage Scaling)
@ 2012-04-26 17:40 Keerthy
  2012-04-26 17:40 ` [PATCH V3 01/10] ARM: OMAP2+: SmartReflex: move the smartreflex header to include/linux/power Keerthy
                   ` (11 more replies)
  0 siblings, 12 replies; 49+ messages in thread
From: Keerthy @ 2012-04-26 17:40 UTC (permalink / raw)
  To: linux-omap, linux-arm-kernel, khilman, rjw, linux-kernel, linux-pm
  Cc: j-pihet, j-keerthy

From: J Keerthy <j-keerthy@ti.com>

AVS(Adaptive Voltage Scaling) is a power management technique which
controls the operating voltage of a device in order to optimize (i.e. reduce)
its power consumption. The voltage is adapted depending on static factors
(chip manufacturing process) and dynamic factors (temperature
depending performance).
The TI AVS solution is named Smartreflex. 

To that end, create the AVS driver in drivers/power/avs and
move the OMAP SmartReflex code to the new directory. The
class driver is still retained in the mach-omap2 directory.

In preparation to the move of the OMAP code the following changes have been
made:
- fill in platform data from the device initialization code and pass
 it to the driver,
- create CONFIG_AVS* config options accordingly.

The platform integration data for SmartReflex is passed from hwmod
and the voltage layer to the driver using pdata.

Tested on OMAP4430 SDP using omap2plus_defconfig with the
CONFIG_POWER_AVS* options set. Voltage correction seen
on oscilloscope on all three VDDs.
Based on master branch of the l-o git tree, commit
6bd61e8de0511dd9831eb9d89eea0b4603a10e9e.

Tree: git://gitorious.org/~keerthy05/omap-pm/keerthy-sr.git for_smartreflex_ip_driver_move 

V3: rework after the comments on MLs
 . Retain Efuse offsets check to identify a particular OPP.
 . Introduce a common header file accessible both by arch/arm/mach-omap2/
   and drivers/.
 . Retain the class driver in mach-omap2/ and move IP driver to drivers/power/
   avs since class driver needs voltage layer support.

History:
v2: rework after the comments on MLs
 . Keep the OMAP Kconfig options in the arch dir (Rafael),
 . Move the shared header file from plat-omap to
   include/linux/power/ (Tony)

v1: initial revision

J Keerthy (1):
  ARM: OMAP2+: Voltage: Move the omap_volt_data structure to plat

Jean Pihet (9):
  ARM: OMAP2+: SmartReflex: move the smartreflex header to include/linux/power
  ARM: OMAP3+: SmartReflex: class drivers should use struct omap_sr *
  ARM: OMAP2+: smartreflex: Use the names from hwmod data instead of
    voltage domains.
  ARM: OMAP3: hwmod: rename the smartreflex entries
  ARM: OMAP2+: SmartReflex: introduce a busy loop condition test macro
  ARM: OMAP2+: SmartReflex: Use per-OPP data structure
  ARM: OMAP2+: SmartReflex: Create per-opp debugfs node for errminlimit
  ARM: OMAP2+: SmartReflex: add POWER_AVS Kconfig options
  ARM: OMAP: SmartReflex: Move smartreflex driver to drivers/
 arch/arm/mach-omap2/Makefile                       |    5 +-
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c         |   12 +-
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c         |    3 +-
 arch/arm/mach-omap2/pm.h                           |    2 +-
 arch/arm/mach-omap2/smartreflex-class3.c           |   29 ++--
 arch/arm/mach-omap2/sr_device.c                    |   39 ++++-
 arch/arm/mach-omap2/voltage.h                      |   21 +---
 arch/arm/plat-omap/Kconfig                         |   31 ++--
 arch/arm/plat-omap/include/plat/voltage.h          |   21 +++-
 drivers/power/Kconfig                              |    2 +
 drivers/power/Makefile                             |    1 +
 drivers/power/avs/Kconfig                          |   12 ++
 drivers/power/avs/Makefile                         |    1 +
 .../mach-omap2 => drivers/power/avs}/smartreflex.c |  161 ++++++++------------
 .../linux/power}/smartreflex.h                     |   74 ++++++++--
 15 files changed, 235 insertions(+), 179 deletions(-)
 create mode 100644 drivers/power/avs/Kconfig
 create mode 100644 drivers/power/avs/Makefile
 rename {arch/arm/mach-omap2 => drivers/power/avs}/smartreflex.c (90%)
 rename {arch/arm/mach-omap2 => include/linux/power}/smartreflex.h (79%)

-- 
1.7.5.4


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

* [PATCH V3 01/10] ARM: OMAP2+: SmartReflex: move the smartreflex header to include/linux/power
  2012-04-26 17:40 [PATCH V3 00/10] PM: Create the AVS(Adaptive Voltage Scaling) Keerthy
@ 2012-04-26 17:40 ` Keerthy
  2012-04-26 17:40 ` [PATCH V3 02/10] ARM: OMAP3+: SmartReflex: class drivers should use struct omap_sr * Keerthy
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 49+ messages in thread
From: Keerthy @ 2012-04-26 17:40 UTC (permalink / raw)
  To: linux-omap, linux-arm-kernel, khilman, rjw, linux-kernel, linux-pm
  Cc: j-pihet, j-keerthy

From: Jean Pihet <j-pihet@ti.com>

Move the smartreflex header file
(arch/arm/mach-omap2/smartreflex.h) in a new header file
include/linux/power/smartreflex.h.

This change makes the SmartReflex implementation ready for the move
to drivers/.

Signed-off-by: Jean Pihet <j-pihet@ti.com>
Signed-off-by: J Keerthy <j-keerthy@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c         |    4 ++--
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c         |    3 +--
 arch/arm/mach-omap2/smartreflex-class3.c           |    3 ++-
 arch/arm/mach-omap2/smartreflex.c                  |    3 +--
 arch/arm/mach-omap2/sr_device.c                    |    2 +-
 .../linux/power}/smartreflex.h                     |    7 ++++---
 6 files changed, 11 insertions(+), 11 deletions(-)
 rename {arch/arm/mach-omap2 => include/linux/power}/smartreflex.h (98%)

diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
index 0c65079..144d118 100644
--- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
@@ -14,6 +14,8 @@
  *
  * XXX these should be marked initdata for multi-OMAP kernels
  */
+#include <linux/power/smartreflex.h>
+
 #include <plat/omap_hwmod.h>
 #include <mach/irqs.h>
 #include <plat/cpu.h>
@@ -29,8 +31,6 @@
 #include <plat/dmtimer.h>
 
 #include "omap_hwmod_common_data.h"
-
-#include "smartreflex.h"
 #include "prm-regbits-34xx.h"
 #include "cm-regbits-34xx.h"
 #include "wd_timer.h"
diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
index 4906129..1079c79 100644
--- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
@@ -19,6 +19,7 @@
  */
 
 #include <linux/io.h>
+#include <linux/power/smartreflex.h>
 
 #include <plat/omap_hwmod.h>
 #include <plat/cpu.h>
@@ -32,8 +33,6 @@
 #include <plat/common.h>
 
 #include "omap_hwmod_common_data.h"
-
-#include "smartreflex.h"
 #include "cm1_44xx.h"
 #include "cm2_44xx.h"
 #include "prm44xx.h"
diff --git a/arch/arm/mach-omap2/smartreflex-class3.c b/arch/arm/mach-omap2/smartreflex-class3.c
index 955566e..ab8cf83 100644
--- a/arch/arm/mach-omap2/smartreflex-class3.c
+++ b/arch/arm/mach-omap2/smartreflex-class3.c
@@ -11,7 +11,8 @@
  * published by the Free Software Foundation.
  */
 
-#include "smartreflex.h"
+#include <linux/power/smartreflex.h>
+#include "voltage.h"
 
 static int sr_class3_enable(struct voltagedomain *voltdm)
 {
diff --git a/arch/arm/mach-omap2/smartreflex.c b/arch/arm/mach-omap2/smartreflex.c
index 008fbd7..98309d3 100644
--- a/arch/arm/mach-omap2/smartreflex.c
+++ b/arch/arm/mach-omap2/smartreflex.c
@@ -25,11 +25,10 @@
 #include <linux/delay.h>
 #include <linux/slab.h>
 #include <linux/pm_runtime.h>
+#include <linux/power/smartreflex.h>
 
 #include "common.h"
-
 #include "pm.h"
-#include "smartreflex.h"
 
 #define SMARTREFLEX_NAME_LEN	16
 #define NVALUE_NAME_LEN		40
diff --git a/arch/arm/mach-omap2/sr_device.c b/arch/arm/mach-omap2/sr_device.c
index a503e1e..86e438e 100644
--- a/arch/arm/mach-omap2/sr_device.c
+++ b/arch/arm/mach-omap2/sr_device.c
@@ -17,6 +17,7 @@
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
  */
+#include <linux/power/smartreflex.h>
 
 #include <linux/err.h>
 #include <linux/slab.h>
@@ -24,7 +25,6 @@
 
 #include <plat/omap_device.h>
 
-#include "smartreflex.h"
 #include "voltage.h"
 #include "control.h"
 #include "pm.h"
diff --git a/arch/arm/mach-omap2/smartreflex.h b/include/linux/power/smartreflex.h
similarity index 98%
rename from arch/arm/mach-omap2/smartreflex.h
rename to include/linux/power/smartreflex.h
index 5809141..69eb270 100644
--- a/arch/arm/mach-omap2/smartreflex.h
+++ b/include/linux/power/smartreflex.h
@@ -17,12 +17,13 @@
  * published by the Free Software Foundation.
  */
 
-#ifndef __ASM_ARM_OMAP_SMARTREFLEX_H
-#define __ASM_ARM_OMAP_SMARTREFLEX_H
+#ifndef __POWER_SMARTREFLEX_H
+#define __POWER_SMARTREFLEX_H
 
+#include <linux/types.h>
 #include <linux/platform_device.h>
 
-#include "voltage.h"
+#include <plat/voltage.h>
 
 /*
  * Different Smartreflex IPs version. The v1 is the 65nm version used in
-- 
1.7.5.4


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

* [PATCH V3 02/10] ARM: OMAP3+: SmartReflex: class drivers should use struct omap_sr *
  2012-04-26 17:40 [PATCH V3 00/10] PM: Create the AVS(Adaptive Voltage Scaling) Keerthy
  2012-04-26 17:40 ` [PATCH V3 01/10] ARM: OMAP2+: SmartReflex: move the smartreflex header to include/linux/power Keerthy
@ 2012-04-26 17:40 ` Keerthy
  2012-04-26 17:40 ` [PATCH V3 03/10] ARM: OMAP2+: smartreflex: Use the names from hwmod data instead of voltage domains Keerthy
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 49+ messages in thread
From: Keerthy @ 2012-04-26 17:40 UTC (permalink / raw)
  To: linux-omap, linux-arm-kernel, khilman, rjw, linux-kernel, linux-pm
  Cc: j-pihet, j-keerthy, Paul Walmsley, Thara Gopinath, Nishanth Menon

From: Jean Pihet <j-pihet@ti.com>

Convert SmartReflex "class" functions to take a struct omap_sr *, rather than
a struct voltagedomain *.  SmartReflex code should be driver code and not
tightly coupled to OMAP subarchitecture-specific structures.

Based on Paul's original code for the SmartReflex driver conversion.

Signed-off-by: Jean Pihet <j-pihet@ti.com>
Signed-off-by: J Keerthy <j-keerthy@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Thara Gopinath <thara@ti.com>
Cc: Nishanth Menon <nm@ti.com>
Cc: Kevin Hilman <khilman@ti.com>
---
 arch/arm/mach-omap2/smartreflex-class3.c |   24 +++++++++---------
 arch/arm/mach-omap2/smartreflex.c        |   37 +++++------------------------
 include/linux/power/smartreflex.h        |   31 +++++++++++++++++++++---
 3 files changed, 46 insertions(+), 46 deletions(-)

diff --git a/arch/arm/mach-omap2/smartreflex-class3.c b/arch/arm/mach-omap2/smartreflex-class3.c
index ab8cf83..9381654 100644
--- a/arch/arm/mach-omap2/smartreflex-class3.c
+++ b/arch/arm/mach-omap2/smartreflex-class3.c
@@ -14,34 +14,34 @@
 #include <linux/power/smartreflex.h>
 #include "voltage.h"
 
-static int sr_class3_enable(struct voltagedomain *voltdm)
+static int sr_class3_enable(struct omap_sr *sr)
 {
-	unsigned long volt = voltdm_get_voltage(voltdm);
+	unsigned long volt = voltdm_get_voltage(sr->voltdm);
 
 	if (!volt) {
 		pr_warning("%s: Curr voltage unknown. Cannot enable sr_%s\n",
-				__func__, voltdm->name);
+				__func__, sr->voltdm->name);
 		return -ENODATA;
 	}
 
-	omap_vp_enable(voltdm);
-	return sr_enable(voltdm, volt);
+	omap_vp_enable(sr->voltdm);
+	return sr_enable(sr->voltdm, volt);
 }
 
-static int sr_class3_disable(struct voltagedomain *voltdm, int is_volt_reset)
+static int sr_class3_disable(struct omap_sr *sr, int is_volt_reset)
 {
-	sr_disable_errgen(voltdm);
-	omap_vp_disable(voltdm);
-	sr_disable(voltdm);
+	sr_disable_errgen(sr->voltdm);
+	omap_vp_disable(sr->voltdm);
+	sr_disable(sr->voltdm);
 	if (is_volt_reset)
-		voltdm_reset(voltdm);
+		voltdm_reset(sr->voltdm);
 
 	return 0;
 }
 
-static int sr_class3_configure(struct voltagedomain *voltdm)
+static int sr_class3_configure(struct omap_sr *sr)
 {
-	return sr_configure_errgen(voltdm);
+	return sr_configure_errgen(sr->voltdm);
 }
 
 /* SR class3 structure */
diff --git a/arch/arm/mach-omap2/smartreflex.c b/arch/arm/mach-omap2/smartreflex.c
index 98309d3..82bdd28 100644
--- a/arch/arm/mach-omap2/smartreflex.c
+++ b/arch/arm/mach-omap2/smartreflex.c
@@ -34,29 +34,6 @@
 #define NVALUE_NAME_LEN		40
 #define SR_DISABLE_TIMEOUT	200
 
-struct omap_sr {
-	struct list_head		node;
-	struct platform_device		*pdev;
-	struct omap_sr_nvalue_table	*nvalue_table;
-	struct voltagedomain		*voltdm;
-	struct dentry			*dbg_dir;
-	unsigned int			irq;
-	int				srid;
-	int				ip_type;
-	int				nvalue_count;
-	bool				autocomp_active;
-	u32				clk_length;
-	u32				err_weight;
-	u32				err_minlimit;
-	u32				err_maxlimit;
-	u32				accum_data;
-	u32				senn_avgweight;
-	u32				senp_avgweight;
-	u32				senp_mod;
-	u32				senn_mod;
-	void __iomem			*base;
-};
-
 /* sr_list contains all the instances of smartreflex module */
 static LIST_HEAD(sr_list);
 
@@ -147,7 +124,7 @@ static irqreturn_t sr_interrupt(int irq, void *data)
 	}
 
 	if (sr_class->notify)
-		sr_class->notify(sr_info->voltdm, status);
+		sr_class->notify(sr_info, status);
 
 	return IRQ_HANDLED;
 }
@@ -225,7 +202,7 @@ static void sr_start_vddautocomp(struct omap_sr *sr)
 		return;
 	}
 
-	if (!sr_class->enable(sr->voltdm))
+	if (!sr_class->enable(sr))
 		sr->autocomp_active = true;
 }
 
@@ -239,7 +216,7 @@ static void sr_stop_vddautocomp(struct omap_sr *sr)
 	}
 
 	if (sr->autocomp_active) {
-		sr_class->disable(sr->voltdm, 1);
+		sr_class->disable(sr, 1);
 		sr->autocomp_active = false;
 	}
 }
@@ -654,7 +631,7 @@ int sr_enable(struct voltagedomain *voltdm, unsigned long volt)
 		return 0;
 
 	/* Configure SR */
-	ret = sr_class->configure(voltdm);
+	ret = sr_class->configure(sr);
 	if (ret)
 		return ret;
 
@@ -772,7 +749,7 @@ void omap_sr_enable(struct voltagedomain *voltdm)
 		return;
 	}
 
-	sr_class->enable(voltdm);
+	sr_class->enable(sr);
 }
 
 /**
@@ -805,7 +782,7 @@ void omap_sr_disable(struct voltagedomain *voltdm)
 		return;
 	}
 
-	sr_class->disable(voltdm, 0);
+	sr_class->disable(sr, 0);
 }
 
 /**
@@ -838,7 +815,7 @@ void omap_sr_disable_reset_volt(struct voltagedomain *voltdm)
 		return;
 	}
 
-	sr_class->disable(voltdm, 1);
+	sr_class->disable(sr, 1);
 }
 
 /**
diff --git a/include/linux/power/smartreflex.h b/include/linux/power/smartreflex.h
index 69eb270..4224698 100644
--- a/include/linux/power/smartreflex.h
+++ b/include/linux/power/smartreflex.h
@@ -143,6 +143,29 @@
 #define OMAP3430_SR_ERRWEIGHT		0x04
 #define OMAP3430_SR_ERRMAXLIMIT		0x02
 
+struct omap_sr {
+	struct list_head		node;
+	struct platform_device		*pdev;
+	struct omap_sr_nvalue_table	*nvalue_table;
+	struct voltagedomain		*voltdm;
+	struct dentry			*dbg_dir;
+	unsigned int			irq;
+	int				srid;
+	int				ip_type;
+	int				nvalue_count;
+	bool				autocomp_active;
+	u32				clk_length;
+	u32				err_weight;
+	u32				err_minlimit;
+	u32				err_maxlimit;
+	u32				accum_data;
+	u32				senn_avgweight;
+	u32				senp_avgweight;
+	u32				senp_mod;
+	u32				senn_mod;
+	void __iomem			*base;
+};
+
 /**
  * struct omap_sr_pmic_data - Strucutre to be populated by pmic code to pass
  *				pmic specific info to smartreflex driver
@@ -187,10 +210,10 @@ struct omap_smartreflex_dev_attr {
  *			based decisions.
  */
 struct omap_sr_class_data {
-	int (*enable)(struct voltagedomain *voltdm);
-	int (*disable)(struct voltagedomain *voltdm, int is_volt_reset);
-	int (*configure)(struct voltagedomain *voltdm);
-	int (*notify)(struct voltagedomain *voltdm, u32 status);
+	int (*enable)(struct omap_sr *sr);
+	int (*disable)(struct omap_sr *sr, int is_volt_reset);
+	int (*configure)(struct omap_sr *sr);
+	int (*notify)(struct omap_sr *sr, u32 status);
 	u8 notify_flags;
 	u8 class_type;
 };
-- 
1.7.5.4


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

* [PATCH V3 03/10] ARM: OMAP2+: smartreflex: Use the names from hwmod data instead of voltage domains.
  2012-04-26 17:40 [PATCH V3 00/10] PM: Create the AVS(Adaptive Voltage Scaling) Keerthy
  2012-04-26 17:40 ` [PATCH V3 01/10] ARM: OMAP2+: SmartReflex: move the smartreflex header to include/linux/power Keerthy
  2012-04-26 17:40 ` [PATCH V3 02/10] ARM: OMAP3+: SmartReflex: class drivers should use struct omap_sr * Keerthy
@ 2012-04-26 17:40 ` Keerthy
  2012-04-26 17:40 ` [PATCH V3 04/10] ARM: OMAP3: hwmod: rename the smartreflex entries Keerthy
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 49+ messages in thread
From: Keerthy @ 2012-04-26 17:40 UTC (permalink / raw)
  To: linux-omap, linux-arm-kernel, khilman, rjw, linux-kernel, linux-pm
  Cc: j-pihet, j-keerthy

From: Jean Pihet <j-pihet@ti.com>

Associate a name with each SmartReflex instance from the hwmod data,
rather than attempting to reuse the name of a voltage domain. The name
from hwmod better reflects the smartreflex integration in the system.

Also have the name passed to the drivers using pdata, which helps to remove
any dependencies on SoC-specific structures.

Signed-off-by: Jean Pihet <j-pihet@ti.com>
Signed-off-by: J Keerthy <j-keerthy@ti.com>
---
 arch/arm/mach-omap2/smartreflex-class3.c |    4 +-
 arch/arm/mach-omap2/smartreflex.c        |   65 ++++++++++++------------------
 arch/arm/mach-omap2/sr_device.c          |    1 +
 include/linux/power/smartreflex.h        |    3 +
 4 files changed, 32 insertions(+), 41 deletions(-)

diff --git a/arch/arm/mach-omap2/smartreflex-class3.c b/arch/arm/mach-omap2/smartreflex-class3.c
index 9381654..1da8f03 100644
--- a/arch/arm/mach-omap2/smartreflex-class3.c
+++ b/arch/arm/mach-omap2/smartreflex-class3.c
@@ -19,8 +19,8 @@ static int sr_class3_enable(struct omap_sr *sr)
 	unsigned long volt = voltdm_get_voltage(sr->voltdm);
 
 	if (!volt) {
-		pr_warning("%s: Curr voltage unknown. Cannot enable sr_%s\n",
-				__func__, sr->voltdm->name);
+		pr_warning("%s: Curr voltage unknown. Cannot enable %s\n",
+				__func__, sr->name);
 		return -ENODATA;
 	}
 
diff --git a/arch/arm/mach-omap2/smartreflex.c b/arch/arm/mach-omap2/smartreflex.c
index 82bdd28..2edd1e2 100644
--- a/arch/arm/mach-omap2/smartreflex.c
+++ b/arch/arm/mach-omap2/smartreflex.c
@@ -183,7 +183,7 @@ static void sr_set_regfields(struct omap_sr *sr)
 		sr->err_weight = OMAP3430_SR_ERRWEIGHT;
 		sr->err_maxlimit = OMAP3430_SR_ERRMAXLIMIT;
 		sr->accum_data = OMAP3430_SR_ACCUMDATA;
-		if (!(strcmp(sr->voltdm->name, "mpu"))) {
+		if (!(strcmp(sr->name, "sr1"))) {
 			sr->senn_avgweight = OMAP3430_SR1_SENNAVGWEIGHT;
 			sr->senp_avgweight = OMAP3430_SR1_SENPAVGWEIGHT;
 		} else {
@@ -234,19 +234,13 @@ static void sr_stop_vddautocomp(struct omap_sr *sr)
  */
 static int sr_late_init(struct omap_sr *sr_info)
 {
-	char *name;
 	struct omap_sr_data *pdata = sr_info->pdev->dev.platform_data;
 	struct resource *mem;
 	int ret = 0;
 
 	if (sr_class->notify && sr_class->notify_flags && sr_info->irq) {
-		name = kasprintf(GFP_KERNEL, "sr_%s", sr_info->voltdm->name);
-		if (name == NULL) {
-			ret = -ENOMEM;
-			goto error;
-		}
 		ret = request_irq(sr_info->irq, sr_interrupt,
-				0, name, sr_info);
+				  0, sr_info->name, sr_info);
 		if (ret)
 			goto error;
 		disable_irq(sr_info->irq);
@@ -265,7 +259,6 @@ error:
 	dev_err(&sr_info->pdev->dev, "%s: ERROR in registering"
 		"interrupt handler. Smartreflex will"
 		"not function as desired\n", __func__);
-	kfree(name);
 	kfree(sr_info);
 
 	return ret;
@@ -395,8 +388,7 @@ int sr_configure_errgen(struct voltagedomain *voltdm)
 	struct omap_sr *sr = _sr_lookup(voltdm);
 
 	if (IS_ERR(sr)) {
-		pr_warning("%s: omap_sr struct for sr_%s not found\n",
-			__func__, voltdm->name);
+		pr_warning("%s: omap_sr struct for voltdm not found\n",	__func__);
 		return PTR_ERR(sr);
 	}
 
@@ -463,8 +455,7 @@ int sr_disable_errgen(struct voltagedomain *voltdm)
 	struct omap_sr *sr = _sr_lookup(voltdm);
 
 	if (IS_ERR(sr)) {
-		pr_warning("%s: omap_sr struct for sr_%s not found\n",
-			__func__, voltdm->name);
+		pr_warning("%s: omap_sr struct for voltdm not found\n",	__func__);
 		return PTR_ERR(sr);
 	}
 
@@ -514,8 +505,7 @@ int sr_configure_minmax(struct voltagedomain *voltdm)
 	struct omap_sr *sr = _sr_lookup(voltdm);
 
 	if (IS_ERR(sr)) {
-		pr_warning("%s: omap_sr struct for sr_%s not found\n",
-			__func__, voltdm->name);
+		pr_warning("%s: omap_sr struct for voltdm not found\n",	__func__);
 		return PTR_ERR(sr);
 	}
 
@@ -600,8 +590,7 @@ int sr_enable(struct voltagedomain *voltdm, unsigned long volt)
 	int ret;
 
 	if (IS_ERR(sr)) {
-		pr_warning("%s: omap_sr struct for sr_%s not found\n",
-			__func__, voltdm->name);
+		pr_warning("%s: omap_sr struct for voltdm not found\n",	__func__);
 		return PTR_ERR(sr);
 	}
 
@@ -654,8 +643,7 @@ void sr_disable(struct voltagedomain *voltdm)
 	struct omap_sr *sr = _sr_lookup(voltdm);
 
 	if (IS_ERR(sr)) {
-		pr_warning("%s: omap_sr struct for sr_%s not found\n",
-			__func__, voltdm->name);
+		pr_warning("%s: omap_sr struct for voltdm not found\n",	__func__);
 		return;
 	}
 
@@ -735,8 +723,7 @@ void omap_sr_enable(struct voltagedomain *voltdm)
 	struct omap_sr *sr = _sr_lookup(voltdm);
 
 	if (IS_ERR(sr)) {
-		pr_warning("%s: omap_sr struct for sr_%s not found\n",
-			__func__, voltdm->name);
+		pr_warning("%s: omap_sr struct for voltdm not found\n",	__func__);
 		return;
 	}
 
@@ -768,8 +755,7 @@ void omap_sr_disable(struct voltagedomain *voltdm)
 	struct omap_sr *sr = _sr_lookup(voltdm);
 
 	if (IS_ERR(sr)) {
-		pr_warning("%s: omap_sr struct for sr_%s not found\n",
-			__func__, voltdm->name);
+		pr_warning("%s: omap_sr struct for voltdm not found\n",	__func__);
 		return;
 	}
 
@@ -801,8 +787,7 @@ void omap_sr_disable_reset_volt(struct voltagedomain *voltdm)
 	struct omap_sr *sr = _sr_lookup(voltdm);
 
 	if (IS_ERR(sr)) {
-		pr_warning("%s: omap_sr struct for sr_%s not found\n",
-			__func__, voltdm->name);
+		pr_warning("%s: omap_sr struct for voltdm not found\n",	__func__);
 		return;
 	}
 
@@ -889,7 +874,6 @@ static int __init omap_sr_probe(struct platform_device *pdev)
 	struct dentry *nvalue_dir;
 	struct omap_volt_data *volt_data;
 	int i, ret = 0;
-	char *name;
 
 	sr_info = kzalloc(sizeof(struct omap_sr), GFP_KERNEL);
 	if (!sr_info) {
@@ -926,6 +910,14 @@ static int __init omap_sr_probe(struct platform_device *pdev)
 	pm_runtime_enable(&pdev->dev);
 	pm_runtime_irq_safe(&pdev->dev);
 
+	sr_info->name = kasprintf(GFP_KERNEL, "%s", pdata->name);
+	if (!sr_info->name) {
+		dev_err(&pdev->dev, "%s: Unable to alloc SR instance name\n",
+			__func__);
+		ret = -ENOMEM;
+		goto err_release_region;
+	}
+
 	sr_info->pdev = pdev;
 	sr_info->srid = pdev->id;
 	sr_info->voltdm = pdata->voltdm;
@@ -973,20 +965,12 @@ static int __init omap_sr_probe(struct platform_device *pdev)
 		}
 	}
 
-	name = kasprintf(GFP_KERNEL, "sr_%s", sr_info->voltdm->name);
-	if (!name) {
-		dev_err(&pdev->dev, "%s: Unable to alloc debugfs name\n",
-			__func__);
-		ret = -ENOMEM;
-		goto err_iounmap;
-	}
-	sr_info->dbg_dir = debugfs_create_dir(name, sr_dbg_dir);
-	kfree(name);
+	sr_info->dbg_dir = debugfs_create_dir(sr_info->name, sr_dbg_dir);
 	if (IS_ERR_OR_NULL(sr_info->dbg_dir)) {
 		dev_err(&pdev->dev, "%s: Unable to create debugfs directory\n",
 			__func__);
 		ret = PTR_ERR(sr_info->dbg_dir);
-		goto err_iounmap;
+		goto err_free_name;
 	}
 
 	(void) debugfs_create_file("autocomp", S_IRUGO | S_IWUSR,
@@ -1008,10 +992,10 @@ static int __init omap_sr_probe(struct platform_device *pdev)
 
 	omap_voltage_get_volttable(sr_info->voltdm, &volt_data);
 	if (!volt_data) {
-		dev_warn(&pdev->dev, "%s: No Voltage table for the"
-			" corresponding vdd vdd_%s. Cannot create debugfs"
+		dev_warn(&pdev->dev, "%s: %s: No Voltage table for the"
+			" corresponding vdd. Cannot create debugfs"
 			"entries for n-values\n",
-			__func__, sr_info->voltdm->name);
+			__func__, sr_info->name);
 		ret = -ENODATA;
 		goto err_debugfs;
 	}
@@ -1029,6 +1013,8 @@ static int __init omap_sr_probe(struct platform_device *pdev)
 
 err_debugfs:
 	debugfs_remove_recursive(sr_info->dbg_dir);
+err_free_name:
+	kfree(sr_info->name);
 err_iounmap:
 	list_del(&sr_info->node);
 	iounmap(sr_info->base);
@@ -1065,6 +1051,7 @@ static int __devexit omap_sr_remove(struct platform_device *pdev)
 
 	list_del(&sr_info->node);
 	iounmap(sr_info->base);
+	kfree(sr_info->name);
 	kfree(sr_info);
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	release_mem_region(mem->start, resource_size(mem));
diff --git a/arch/arm/mach-omap2/sr_device.c b/arch/arm/mach-omap2/sr_device.c
index 86e438e..e081174 100644
--- a/arch/arm/mach-omap2/sr_device.c
+++ b/arch/arm/mach-omap2/sr_device.c
@@ -93,6 +93,7 @@ static int __init sr_dev_init(struct omap_hwmod *oh, void *user)
 		goto exit;
 	}
 
+	sr_data->name = oh->name;
 	sr_data->ip_type = oh->class->rev;
 	sr_data->senn_mod = 0x1;
 	sr_data->senp_mod = 0x1;
diff --git a/include/linux/power/smartreflex.h b/include/linux/power/smartreflex.h
index 4224698..884eaee 100644
--- a/include/linux/power/smartreflex.h
+++ b/include/linux/power/smartreflex.h
@@ -144,6 +144,7 @@
 #define OMAP3430_SR_ERRMAXLIMIT		0x02
 
 struct omap_sr {
+	char				*name;
 	struct list_head		node;
 	struct platform_device		*pdev;
 	struct omap_sr_nvalue_table	*nvalue_table;
@@ -232,6 +233,7 @@ struct omap_sr_nvalue_table {
 /**
  * struct omap_sr_data - Smartreflex platform data.
  *
+ * @name:		instance name
  * @ip_type:		Smartreflex IP type.
  * @senp_mod:		SENPENABLE value for the sr
  * @senn_mod:		SENNENABLE value for sr
@@ -243,6 +245,7 @@ struct omap_sr_nvalue_table {
  * @voltdm:		Pointer to the voltage domain associated with the SR
  */
 struct omap_sr_data {
+	const char			*name;
 	int				ip_type;
 	u32				senp_mod;
 	u32				senn_mod;
-- 
1.7.5.4


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

* [PATCH V3 04/10] ARM: OMAP3: hwmod: rename the smartreflex entries
  2012-04-26 17:40 [PATCH V3 00/10] PM: Create the AVS(Adaptive Voltage Scaling) Keerthy
                   ` (2 preceding siblings ...)
  2012-04-26 17:40 ` [PATCH V3 03/10] ARM: OMAP2+: smartreflex: Use the names from hwmod data instead of voltage domains Keerthy
@ 2012-04-26 17:40 ` Keerthy
  2012-05-04  8:30   ` AnilKumar, Chimata
  2012-04-26 17:40 ` [PATCH V3 05/10] ARM: OMAP2+: SmartReflex: introduce a busy loop condition test macro Keerthy
                   ` (7 subsequent siblings)
  11 siblings, 1 reply; 49+ messages in thread
From: Keerthy @ 2012-04-26 17:40 UTC (permalink / raw)
  To: linux-omap, linux-arm-kernel, khilman, rjw, linux-kernel, linux-pm
  Cc: j-pihet, j-keerthy

From: Jean Pihet <j-pihet@ti.com>

Change the name field value to better reflect the smartreflex
integration in the system.

Signed-off-by: Jean Pihet <j-pihet@ti.com>
Signed-off-by: J Keerthy <j-keerthy@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |    8 ++++----
 arch/arm/mach-omap2/smartreflex.c          |    2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
index 144d118..15907b0 100644
--- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
@@ -1324,7 +1324,7 @@ static struct omap_hwmod_irq_info omap3_smartreflex_mpu_irqs[] = {
 };
 
 static struct omap_hwmod omap34xx_sr1_hwmod = {
-	.name		= "sr1",
+	.name		= "smartreflex_mpu_iva",
 	.class		= &omap34xx_smartreflex_hwmod_class,
 	.main_clk	= "sr1_fck",
 	.prcm		= {
@@ -1342,7 +1342,7 @@ static struct omap_hwmod omap34xx_sr1_hwmod = {
 };
 
 static struct omap_hwmod omap36xx_sr1_hwmod = {
-	.name		= "sr1",
+	.name		= "smartreflex_mpu_iva",
 	.class		= &omap36xx_smartreflex_hwmod_class,
 	.main_clk	= "sr1_fck",
 	.prcm		= {
@@ -1369,7 +1369,7 @@ static struct omap_hwmod_irq_info omap3_smartreflex_core_irqs[] = {
 };
 
 static struct omap_hwmod omap34xx_sr2_hwmod = {
-	.name		= "sr2",
+	.name		= "smartreflex_core",
 	.class		= &omap34xx_smartreflex_hwmod_class,
 	.main_clk	= "sr2_fck",
 	.prcm		= {
@@ -1387,7 +1387,7 @@ static struct omap_hwmod omap34xx_sr2_hwmod = {
 };
 
 static struct omap_hwmod omap36xx_sr2_hwmod = {
-	.name		= "sr2",
+	.name		= "smartreflex_core",
 	.class		= &omap36xx_smartreflex_hwmod_class,
 	.main_clk	= "sr2_fck",
 	.prcm		= {
diff --git a/arch/arm/mach-omap2/smartreflex.c b/arch/arm/mach-omap2/smartreflex.c
index 2edd1e2..d859277 100644
--- a/arch/arm/mach-omap2/smartreflex.c
+++ b/arch/arm/mach-omap2/smartreflex.c
@@ -183,7 +183,7 @@ static void sr_set_regfields(struct omap_sr *sr)
 		sr->err_weight = OMAP3430_SR_ERRWEIGHT;
 		sr->err_maxlimit = OMAP3430_SR_ERRMAXLIMIT;
 		sr->accum_data = OMAP3430_SR_ACCUMDATA;
-		if (!(strcmp(sr->name, "sr1"))) {
+		if (!(strcmp(sr->name, "smartreflex_mpu_iva"))) {
 			sr->senn_avgweight = OMAP3430_SR1_SENNAVGWEIGHT;
 			sr->senp_avgweight = OMAP3430_SR1_SENPAVGWEIGHT;
 		} else {
-- 
1.7.5.4


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

* [PATCH V3 05/10] ARM: OMAP2+: SmartReflex: introduce a busy loop condition test macro
  2012-04-26 17:40 [PATCH V3 00/10] PM: Create the AVS(Adaptive Voltage Scaling) Keerthy
                   ` (3 preceding siblings ...)
  2012-04-26 17:40 ` [PATCH V3 04/10] ARM: OMAP3: hwmod: rename the smartreflex entries Keerthy
@ 2012-04-26 17:40 ` Keerthy
  2012-05-04  9:12   ` AnilKumar, Chimata
  2012-04-26 17:40 ` [PATCH V3 06/10] ARM: OMAP2+: Voltage: Move the omap_volt_data structure to plat Keerthy
                   ` (6 subsequent siblings)
  11 siblings, 1 reply; 49+ messages in thread
From: Keerthy @ 2012-04-26 17:40 UTC (permalink / raw)
  To: linux-omap, linux-arm-kernel, khilman, rjw, linux-kernel, linux-pm
  Cc: j-pihet, j-keerthy

From: Jean Pihet <j-pihet@ti.com>

Now that omap_test_timeout is only accessible from mach-omap2/,
introduce a similar function for SR.

This change makes the SmartReflex implementation ready for the move
to drivers/.

Signed-off-by: Jean Pihet <j-pihet@ti.com>
Signed-off-by: J Keerthy <j-keerthy@ti.com>
---
 arch/arm/mach-omap2/smartreflex.c |   12 ++++++------
 include/linux/power/smartreflex.h |   23 ++++++++++++++++++++++-
 2 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mach-omap2/smartreflex.c b/arch/arm/mach-omap2/smartreflex.c
index d859277..acef08d 100644
--- a/arch/arm/mach-omap2/smartreflex.c
+++ b/arch/arm/mach-omap2/smartreflex.c
@@ -289,9 +289,9 @@ static void sr_v1_disable(struct omap_sr *sr)
 	 * Wait for SR to be disabled.
 	 * wait until ERRCONFIG.MCUDISACKINTST = 1. Typical latency is 1us.
 	 */
-	omap_test_timeout((sr_read_reg(sr, ERRCONFIG_V1) &
-			ERRCONFIG_MCUDISACKINTST), SR_DISABLE_TIMEOUT,
-			timeout);
+	sr_test_cond_timeout((sr_read_reg(sr, ERRCONFIG_V1) &
+			     ERRCONFIG_MCUDISACKINTST), SR_DISABLE_TIMEOUT,
+			     timeout);
 
 	if (timeout >= SR_DISABLE_TIMEOUT)
 		dev_warn(&sr->pdev->dev, "%s: Smartreflex disable timedout\n",
@@ -334,9 +334,9 @@ static void sr_v2_disable(struct omap_sr *sr)
 	 * Wait for SR to be disabled.
 	 * wait until IRQSTATUS.MCUDISACKINTST = 1. Typical latency is 1us.
 	 */
-	omap_test_timeout((sr_read_reg(sr, IRQSTATUS) &
-			IRQSTATUS_MCUDISABLEACKINT), SR_DISABLE_TIMEOUT,
-			timeout);
+	sr_test_cond_timeout((sr_read_reg(sr, IRQSTATUS) &
+			     IRQSTATUS_MCUDISABLEACKINT), SR_DISABLE_TIMEOUT,
+			     timeout);
 
 	if (timeout >= SR_DISABLE_TIMEOUT)
 		dev_warn(&sr->pdev->dev, "%s: Smartreflex disable timedout\n",
diff --git a/include/linux/power/smartreflex.h b/include/linux/power/smartreflex.h
index 884eaee..78b795e 100644
--- a/include/linux/power/smartreflex.h
+++ b/include/linux/power/smartreflex.h
@@ -22,7 +22,7 @@
 
 #include <linux/types.h>
 #include <linux/platform_device.h>
-
+#include <linux/delay.h>
 #include <plat/voltage.h>
 
 /*
@@ -168,6 +168,27 @@ struct omap_sr {
 };
 
 /**
+ * test_cond_timeout - busy-loop, testing a condition
+ * @cond: condition to test until it evaluates to true
+ * @timeout: maximum number of microseconds in the timeout
+ * @index: loop index (integer)
+ *
+ * Loop waiting for @cond to become true or until at least @timeout
+ * microseconds have passed.  To use, define some integer @index in the
+ * calling code.  After running, if @index == @timeout, then the loop has
+ * timed out.
+ *
+ * Copied from omap_test_timeout */
+#define sr_test_cond_timeout(cond, timeout, index)		\
+({								\
+	for (index = 0; index < timeout; index++) {		\
+		if (cond)					\
+			break;					\
+		udelay(1);					\
+	}							\
+})
+
+/**
  * struct omap_sr_pmic_data - Strucutre to be populated by pmic code to pass
  *				pmic specific info to smartreflex driver
  *
-- 
1.7.5.4


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

* [PATCH V3 06/10] ARM: OMAP2+: Voltage: Move the omap_volt_data structure to plat
  2012-04-26 17:40 [PATCH V3 00/10] PM: Create the AVS(Adaptive Voltage Scaling) Keerthy
                   ` (4 preceding siblings ...)
  2012-04-26 17:40 ` [PATCH V3 05/10] ARM: OMAP2+: SmartReflex: introduce a busy loop condition test macro Keerthy
@ 2012-04-26 17:40 ` Keerthy
  2012-04-26 17:40 ` [PATCH V3 07/10] ARM: OMAP2+: SmartReflex: Use per-OPP data structure Keerthy
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 49+ messages in thread
From: Keerthy @ 2012-04-26 17:40 UTC (permalink / raw)
  To: linux-omap, linux-arm-kernel, khilman, rjw, linux-kernel, linux-pm
  Cc: j-pihet, j-keerthy

From: J Keerthy <j-keerthy@ti.com>

Move the omap_volt_data structure from mach-omap2/ directory
to arch/arm/plat-omap/include/plat/ so that it is accessible
from both mach-omap2 and drivers directories.

Signed-off-by: J Keerthy <j-keerthy@ti.com>
---
 arch/arm/mach-omap2/voltage.h             |   21 ++-------------------
 arch/arm/plat-omap/include/plat/voltage.h |   21 ++++++++++++++++++++-
 2 files changed, 22 insertions(+), 20 deletions(-)

diff --git a/arch/arm/mach-omap2/voltage.h b/arch/arm/mach-omap2/voltage.h
index 16a1b09..34ef504 100644
--- a/arch/arm/mach-omap2/voltage.h
+++ b/arch/arm/mach-omap2/voltage.h
@@ -16,6 +16,8 @@
 
 #include <linux/err.h>
 
+#include <plat/voltage.h>
+
 #include "vc.h"
 #include "vp.h"
 
@@ -91,25 +93,6 @@ struct voltagedomain {
 };
 
 /**
- * struct omap_volt_data - Omap voltage specific data.
- * @voltage_nominal:	The possible voltage value in uV
- * @sr_efuse_offs:	The offset of the efuse register(from system
- *			control module base address) from where to read
- *			the n-target value for the smartreflex module.
- * @sr_errminlimit:	Error min limit value for smartreflex. This value
- *			differs at differnet opp and thus is linked
- *			with voltage.
- * @vp_errorgain:	Error gain value for the voltage processor. This
- *			field also differs according to the voltage/opp.
- */
-struct omap_volt_data {
-	u32	volt_nominal;
-	u32	sr_efuse_offs;
-	u8	sr_errminlimit;
-	u8	vp_errgain;
-};
-
-/**
  * struct omap_voltdm_pmic - PMIC specific data required by voltage driver.
  * @slew_rate:	PMIC slew rate (in uv/us)
  * @step_size:	PMIC voltage step size (in uv)
diff --git a/arch/arm/plat-omap/include/plat/voltage.h b/arch/arm/plat-omap/include/plat/voltage.h
index 0a6a482..5be4d5d 100644
--- a/arch/arm/plat-omap/include/plat/voltage.h
+++ b/arch/arm/plat-omap/include/plat/voltage.h
@@ -11,10 +11,29 @@
 #ifndef __ARCH_ARM_OMAP_VOLTAGE_H
 #define __ARCH_ARM_OMAP_VOLTAGE_H
 
+/**
+ * struct omap_volt_data - Omap voltage specific data.
+ * @voltage_nominal:	The possible voltage value in uV
+ * @sr_efuse_offs:	The offset of the efuse register(from system
+ *			control module base address) from where to read
+ *			the n-target value for the smartreflex module.
+ * @sr_errminlimit:	Error min limit value for smartreflex. This value
+ *			differs at differnet opp and thus is linked
+ *			with voltage.
+ * @vp_errorgain:	Error gain value for the voltage processor. This
+ *			field also differs according to the voltage/opp.
+ */
+struct omap_volt_data {
+	u32	volt_nominal;
+	u32	sr_efuse_offs;
+	u8	sr_errminlimit;
+	u8	vp_errgain;
+};
 struct voltagedomain;
 
 struct voltagedomain *voltdm_lookup(const char *name);
 int voltdm_scale(struct voltagedomain *voltdm, unsigned long target_volt);
 unsigned long voltdm_get_voltage(struct voltagedomain *voltdm);
-
+struct omap_volt_data *omap_voltage_get_voltdata(struct voltagedomain *voltdm,
+		unsigned long volt);
 #endif
-- 
1.7.5.4


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

* [PATCH V3 07/10] ARM: OMAP2+: SmartReflex: Use per-OPP data structure
  2012-04-26 17:40 [PATCH V3 00/10] PM: Create the AVS(Adaptive Voltage Scaling) Keerthy
                   ` (5 preceding siblings ...)
  2012-04-26 17:40 ` [PATCH V3 06/10] ARM: OMAP2+: Voltage: Move the omap_volt_data structure to plat Keerthy
@ 2012-04-26 17:40 ` Keerthy
  2012-05-10 19:11   ` Guyotte, Greg
  2012-04-26 17:40 ` [PATCH V3 08/10] ARM: OMAP2+: SmartReflex: Create per-opp debugfs node for errminlimit Keerthy
                   ` (4 subsequent siblings)
  11 siblings, 1 reply; 49+ messages in thread
From: Keerthy @ 2012-04-26 17:40 UTC (permalink / raw)
  To: linux-omap, linux-arm-kernel, khilman, rjw, linux-kernel, linux-pm
  Cc: j-pihet, j-keerthy, Paul Walmsley, Thara Gopinath, Nishanth Menon

From: Jean Pihet <j-pihet@ti.com>

The SmartReflex driver incorrectly treats some per-OPP data as data
common to all OPPs (e.g., ERRMINLIMIT).  Move this data into a per-OPP
data structure.

Furthermore, in order to make the SmartReflex implementation ready for
the move to drivers/, remove the dependency from the SR driver code
to the voltage layer by querying the data tables only from the SR device
init code.

Based on Paul's original code for the SmartReflex driver conversion.

Signed-off-by: Jean Pihet <j-pihet@ti.com>
Signed-off-by: J Keerthy <j-keerthy@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Thara Gopinath <thara@ti.com>
Cc: Nishanth Menon <nm@ti.com>
Cc: Kevin Hilman <khilman@ti.com>
---
 arch/arm/mach-omap2/smartreflex.c |   38 +++++++++++++++++-------------------
 arch/arm/mach-omap2/sr_device.c   |   36 +++++++++++++++++++++++++++++-----
 include/linux/power/smartreflex.h |    8 +++++-
 3 files changed, 54 insertions(+), 28 deletions(-)

diff --git a/arch/arm/mach-omap2/smartreflex.c b/arch/arm/mach-omap2/smartreflex.c
index acef08d..20075de 100644
--- a/arch/arm/mach-omap2/smartreflex.c
+++ b/arch/arm/mach-omap2/smartreflex.c
@@ -347,22 +347,23 @@ static void sr_v2_disable(struct omap_sr *sr)
 	sr_write_reg(sr, IRQSTATUS, IRQSTATUS_MCUDISABLEACKINT);
 }
 
-static u32 sr_retrieve_nvalue(struct omap_sr *sr, u32 efuse_offs)
+static struct omap_sr_nvalue_table *sr_retrieve_nvalue_row(
+				struct omap_sr *sr, u32 efuse_offs)
 {
 	int i;
 
 	if (!sr->nvalue_table) {
 		dev_warn(&sr->pdev->dev, "%s: Missing ntarget value table\n",
 			__func__);
-		return 0;
+		return NULL;
 	}
 
 	for (i = 0; i < sr->nvalue_count; i++) {
 		if (sr->nvalue_table[i].efuse_offs == efuse_offs)
-			return sr->nvalue_table[i].nvalue;
+			return &sr->nvalue_table[i];
 	}
 
-	return 0;
+	return NULL;
 }
 
 /* Public Functions */
@@ -586,7 +587,7 @@ int sr_enable(struct voltagedomain *voltdm, unsigned long volt)
 {
 	struct omap_volt_data *volt_data;
 	struct omap_sr *sr = _sr_lookup(voltdm);
-	u32 nvalue_reciprocal;
+	struct omap_sr_nvalue_table *nvalue_row;
 	int ret;
 
 	if (IS_ERR(sr)) {
@@ -602,16 +603,16 @@ int sr_enable(struct voltagedomain *voltdm, unsigned long volt)
 		return PTR_ERR(volt_data);
 	}
 
-	nvalue_reciprocal = sr_retrieve_nvalue(sr, volt_data->sr_efuse_offs);
+	nvalue_row = sr_retrieve_nvalue_row(sr, volt_data->sr_efuse_offs);
 
-	if (!nvalue_reciprocal) {
-		dev_warn(&sr->pdev->dev, "%s: NVALUE = 0 at voltage %ld\n",
-			__func__, volt);
+	if (!nvalue_row) {
+		dev_warn(&sr->pdev->dev, "%s: failure getting SR data for this voltage %ld\n",
+			 __func__, volt);
 		return -ENODATA;
 	}
 
 	/* errminlimit is opp dependent and hence linked to voltage */
-	sr->err_minlimit = volt_data->sr_errminlimit;
+	sr->err_minlimit = nvalue_row->errminlimit;
 
 	pm_runtime_get_sync(&sr->pdev->dev);
 
@@ -624,7 +625,7 @@ int sr_enable(struct voltagedomain *voltdm, unsigned long volt)
 	if (ret)
 		return ret;
 
-	sr_write_reg(sr, NVALUERECIPROCAL, nvalue_reciprocal);
+	sr_write_reg(sr, NVALUERECIPROCAL, nvalue_row->nvalue);
 
 	/* SRCONFIG - enable SR */
 	sr_modify_reg(sr, SRCONFIG, SRCONFIG_SRENABLE, SRCONFIG_SRENABLE);
@@ -872,7 +873,6 @@ static int __init omap_sr_probe(struct platform_device *pdev)
 	struct omap_sr_data *pdata = pdev->dev.platform_data;
 	struct resource *mem, *irq;
 	struct dentry *nvalue_dir;
-	struct omap_volt_data *volt_data;
 	int i, ret = 0;
 
 	sr_info = kzalloc(sizeof(struct omap_sr), GFP_KERNEL);
@@ -990,12 +990,10 @@ static int __init omap_sr_probe(struct platform_device *pdev)
 		goto err_debugfs;
 	}
 
-	omap_voltage_get_volttable(sr_info->voltdm, &volt_data);
-	if (!volt_data) {
-		dev_warn(&pdev->dev, "%s: %s: No Voltage table for the"
-			" corresponding vdd. Cannot create debugfs"
-			"entries for n-values\n",
-			__func__, sr_info->name);
+	if (sr_info->nvalue_count == 0 || !sr_info->nvalue_table) {
+		dev_warn(&pdev->dev, "%s: %s: No Voltage table for the corresponding vdd. Cannot create debugfs entries for n-values\n",
+			 __func__, sr_info->name);
+
 		ret = -ENODATA;
 		goto err_debugfs;
 	}
@@ -1003,8 +1001,8 @@ static int __init omap_sr_probe(struct platform_device *pdev)
 	for (i = 0; i < sr_info->nvalue_count; i++) {
 		char name[NVALUE_NAME_LEN + 1];
 
-		snprintf(name, sizeof(name), "volt_%d",
-			 volt_data[i].volt_nominal);
+		snprintf(name, sizeof(name), "volt_%lu",
+				sr_info->nvalue_table[i].volt_nominal);
 		(void) debugfs_create_x32(name, S_IRUGO | S_IWUSR, nvalue_dir,
 				&(sr_info->nvalue_table[i].nvalue));
 	}
diff --git a/arch/arm/mach-omap2/sr_device.c b/arch/arm/mach-omap2/sr_device.c
index e081174..e107e39 100644
--- a/arch/arm/mach-omap2/sr_device.c
+++ b/arch/arm/mach-omap2/sr_device.c
@@ -36,7 +36,10 @@ static void __init sr_set_nvalues(struct omap_volt_data *volt_data,
 				struct omap_sr_data *sr_data)
 {
 	struct omap_sr_nvalue_table *nvalue_table;
-	int i, count = 0;
+	int i, j, count = 0;
+
+	sr_data->nvalue_count = 0;
+	sr_data->nvalue_table = NULL;
 
 	while (volt_data[count].volt_nominal)
 		count++;
@@ -44,8 +47,14 @@ static void __init sr_set_nvalues(struct omap_volt_data *volt_data,
 	nvalue_table = kzalloc(sizeof(struct omap_sr_nvalue_table)*count,
 			GFP_KERNEL);
 
-	for (i = 0; i < count; i++) {
+	if (!nvalue_table) {
+		pr_err("OMAP: SmartReflex: cannot allocate memory for n-value table\n");
+		return;
+	}
+
+	for (i = 0, j = 0; i < count; i++) {
 		u32 v;
+
 		/*
 		 * In OMAP4 the efuse registers are 24 bit aligned.
 		 * A __raw_readl will fail for non-32 bit aligned address
@@ -58,15 +67,30 @@ static void __init sr_set_nvalues(struct omap_volt_data *volt_data,
 				omap_ctrl_readb(offset + 1) << 8 |
 				omap_ctrl_readb(offset + 2) << 16;
 		} else {
-			 v = omap_ctrl_readl(volt_data[i].sr_efuse_offs);
+			v = omap_ctrl_readl(volt_data[i].sr_efuse_offs);
 		}
 
-		nvalue_table[i].efuse_offs = volt_data[i].sr_efuse_offs;
-		nvalue_table[i].nvalue = v;
+		/*
+		 * Many OMAP SoCs don't have the eFuse values set.
+		 * For example, pretty much all OMAP3xxx before
+		 * ES3.something.
+		 *
+		 * XXX There needs to be some way for board files or
+		 * userspace to add these in.
+		 */
+		if (v == 0)
+			continue;
+
+		nvalue_table[j].nvalue = v;
+		nvalue_table[j].efuse_offs = volt_data[i].sr_efuse_offs;
+		nvalue_table[j].errminlimit = volt_data[i].sr_errminlimit;
+		nvalue_table[j].volt_nominal = volt_data[i].volt_nominal;
+
+		j++;
 	}
 
 	sr_data->nvalue_table = nvalue_table;
-	sr_data->nvalue_count = count;
+	sr_data->nvalue_count = j;
 }
 
 static int __init sr_dev_init(struct omap_hwmod *oh, void *user)
diff --git a/include/linux/power/smartreflex.h b/include/linux/power/smartreflex.h
index 78b795e..222f901 100644
--- a/include/linux/power/smartreflex.h
+++ b/include/linux/power/smartreflex.h
@@ -243,12 +243,16 @@ struct omap_sr_class_data {
 /**
  * struct omap_sr_nvalue_table	- Smartreflex n-target value info
  *
- * @efuse_offs:	The offset of the efuse where n-target values are stored.
- * @nvalue:	The n-target value.
+ * @efuse_offs:	  The offset of the efuse where n-target values are stored.
+ * @nvalue:	  The n-target value.
+ * @errminlimit:  The value of the ERRMINLIMIT bitfield for this n-target
+ * @volt_nominal: microvolts DC that the VDD is initially programmed to
  */
 struct omap_sr_nvalue_table {
 	u32 efuse_offs;
 	u32 nvalue;
+	u32 errminlimit;
+	unsigned long volt_nominal;
 };
 
 /**
-- 
1.7.5.4


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

* [PATCH V3 08/10] ARM: OMAP2+: SmartReflex: Create per-opp debugfs node for errminlimit
  2012-04-26 17:40 [PATCH V3 00/10] PM: Create the AVS(Adaptive Voltage Scaling) Keerthy
                   ` (6 preceding siblings ...)
  2012-04-26 17:40 ` [PATCH V3 07/10] ARM: OMAP2+: SmartReflex: Use per-OPP data structure Keerthy
@ 2012-04-26 17:40 ` Keerthy
  2012-04-26 17:40 ` [PATCH V3 09/10] ARM: OMAP2+: SmartReflex: add POWER_AVS Kconfig options Keerthy
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 49+ messages in thread
From: Keerthy @ 2012-04-26 17:40 UTC (permalink / raw)
  To: linux-omap, linux-arm-kernel, khilman, rjw, linux-kernel, linux-pm
  Cc: j-pihet, j-keerthy

From: Jean Pihet <j-pihet@ti.com>

Remove the global errminlimit debugfs entry and create per-voltage
entries from the data tables.

Signed-off-by: Jean Pihet <j-pihet@ti.com>
Signed-off-by: J Keerthy <j-keerthy@ti.com>
---
 arch/arm/mach-omap2/smartreflex.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/smartreflex.c b/arch/arm/mach-omap2/smartreflex.c
index 20075de..515041c 100644
--- a/arch/arm/mach-omap2/smartreflex.c
+++ b/arch/arm/mach-omap2/smartreflex.c
@@ -979,8 +979,6 @@ static int __init omap_sr_probe(struct platform_device *pdev)
 			&sr_info->err_weight);
 	(void) debugfs_create_x32("errmaxlimit", S_IRUGO, sr_info->dbg_dir,
 			&sr_info->err_maxlimit);
-	(void) debugfs_create_x32("errminlimit", S_IRUGO, sr_info->dbg_dir,
-			&sr_info->err_minlimit);
 
 	nvalue_dir = debugfs_create_dir("nvalue", sr_info->dbg_dir);
 	if (IS_ERR_OR_NULL(nvalue_dir)) {
@@ -1005,6 +1003,11 @@ static int __init omap_sr_probe(struct platform_device *pdev)
 				sr_info->nvalue_table[i].volt_nominal);
 		(void) debugfs_create_x32(name, S_IRUGO | S_IWUSR, nvalue_dir,
 				&(sr_info->nvalue_table[i].nvalue));
+		snprintf(name, sizeof(name), "errminlimit_%lu",
+			 sr_info->nvalue_table[i].volt_nominal);
+		(void) debugfs_create_x32(name, S_IRUGO | S_IWUSR, nvalue_dir,
+				&(sr_info->nvalue_table[i].errminlimit));
+
 	}
 
 	return ret;
-- 
1.7.5.4


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

* [PATCH V3 09/10] ARM: OMAP2+: SmartReflex: add POWER_AVS Kconfig options
  2012-04-26 17:40 [PATCH V3 00/10] PM: Create the AVS(Adaptive Voltage Scaling) Keerthy
                   ` (7 preceding siblings ...)
  2012-04-26 17:40 ` [PATCH V3 08/10] ARM: OMAP2+: SmartReflex: Create per-opp debugfs node for errminlimit Keerthy
@ 2012-04-26 17:40 ` Keerthy
  2012-04-26 17:40 ` [PATCH V3 10/10] ARM: OMAP: SmartReflex: Move smartreflex driver to drivers/ Keerthy
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 49+ messages in thread
From: Keerthy @ 2012-04-26 17:40 UTC (permalink / raw)
  To: linux-omap, linux-arm-kernel, khilman, rjw, linux-kernel, linux-pm
  Cc: j-pihet, j-keerthy

From: Jean Pihet <j-pihet@ti.com>

Add a Kconfig menu (POWER_AVS) and rename the Kconfig options
for the OMAP SmartReflex implementation:
	CONFIG_OMAP_SMARTREFLEX renames to CONFIG_POWER_AVS_OMAP
	CONFIG_OMAP_SMARTREFLEX_CLASS3 renames to CONFIG_POWER_AVS_OMAP_CLASS3

This change makes the SmartReflex implementation ready for the move
to drivers/.

Signed-off-by: Jean Pihet <j-pihet@ti.com>
Signed-off-by: J Keerthy <j-keerthy@ti.com>
---
 arch/arm/mach-omap2/Makefile      |    5 ++-
 arch/arm/mach-omap2/pm.h          |    2 +-
 arch/arm/plat-omap/Kconfig        |   45 ++++++++++++++++++++++++------------
 include/linux/power/smartreflex.h |    2 +-
 4 files changed, 35 insertions(+), 19 deletions(-)

diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
index d8604a3..85ca797 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -69,8 +69,9 @@ obj-$(CONFIG_ARCH_OMAP3)		+= pm34xx.o sleep34xx.o \
 obj-$(CONFIG_ARCH_OMAP4)		+= pm44xx.o omap-mpuss-lowpower.o \
 					   cpuidle44xx.o
 obj-$(CONFIG_PM_DEBUG)			+= pm-debug.o
-obj-$(CONFIG_OMAP_SMARTREFLEX)          += sr_device.o smartreflex.o
-obj-$(CONFIG_OMAP_SMARTREFLEX_CLASS3)	+= smartreflex-class3.o
+
+obj-$(CONFIG_POWER_AVS_OMAP)		+= sr_device.o smartreflex.o
+obj-$(CONFIG_POWER_AVS_OMAP_CLASS3)	+= smartreflex-class3.o
 
 AFLAGS_sleep24xx.o			:=-Wa,-march=armv6
 AFLAGS_sleep34xx.o			:=-Wa,-march=armv7-a$(plus_sec)
diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
index 36fa90b..77c8c85 100644
--- a/arch/arm/mach-omap2/pm.h
+++ b/arch/arm/mach-omap2/pm.h
@@ -109,7 +109,7 @@ extern void enable_omap3630_toggle_l2_on_restore(void);
 static inline void enable_omap3630_toggle_l2_on_restore(void) { }
 #endif		/* defined(CONFIG_PM) && defined(CONFIG_ARCH_OMAP3) */
 
-#ifdef CONFIG_OMAP_SMARTREFLEX
+#ifdef CONFIG_POWER_AVS_OMAP
 extern int omap_devinit_smartreflex(void);
 extern void omap_enable_smartreflex_on_init(void);
 #else
diff --git a/arch/arm/plat-omap/Kconfig b/arch/arm/plat-omap/Kconfig
index ad95c7a..bba384d 100644
--- a/arch/arm/plat-omap/Kconfig
+++ b/arch/arm/plat-omap/Kconfig
@@ -45,37 +45,52 @@ config OMAP_DEBUG_LEDS
 	depends on OMAP_DEBUG_DEVICES
 	default y if LEDS_CLASS
 
-config OMAP_SMARTREFLEX
-	bool "SmartReflex support"
-	depends on (ARCH_OMAP3 || ARCH_OMAP4) && PM
+menuconfig POWER_AVS
+	tristate "Adaptive Voltage Scaling class support"
 	help
-	  Say Y if you want to enable SmartReflex.
+	  AVS(Adaptive Voltage Scaling) is a power management technique which
+	  finely controls the operating voltage of a device in order to optimize
+	   (i.e. reduce) its power consumption.
+	  At a given operating point the voltage is adapted depending on
+	  static factors (chip manufacturing process) and dynamic factors
+	  (temperature depending performance).
+	  AVS is also called SmartReflex on OMAP devices.
+
+	  Say Y here to enable Adaptive Voltage Scaling class support.
+
+if POWER_AVS
 
-	  SmartReflex can perform continuous dynamic voltage
-	  scaling around the nominal operating point voltage
-	  according to silicon characteristics and operating
-	  conditions. Enabling SmartReflex reduces power
-	  consumption.
+config POWER_AVS_OMAP
+	bool "AVS(Adaptive Voltage Scaling) support for OMAP IP versions 1&2"
+	depends on (ARCH_OMAP3 || ARCH_OMAP4) && PM
+	help
+	  Say Y to enable AVS support on OMAP containing the version 1 or
+	  version 2 of the SmartReflex IP.
+	  V1 is the 65nm version used in OMAP3430.
+	  V2 is the update for the 45nm version of the IP used in OMAP3630
+	  and OMAP4430
 
 	  Please note, that by default SmartReflex is only
-	  initialized. To enable the automatic voltage
-	  compensation for vdd mpu  and vdd core from user space,
+	  initialized and not enabled. To enable the automatic voltage
+	  compensation for vdd mpu and vdd core from user space,
 	  user must write 1 to
-		/debug/voltage/vdd_<X>/smartreflex/autocomp,
-	  where X is mpu or core for OMAP3.
+		/debug/smartreflex/sr_<X>/autocomp,
+	  where X is mpu_iva or core for OMAP3.
 	  Optionally autocompensation can be enabled in the kernel
 	  by default during system init via the enable_on_init flag
 	  which an be passed as platform data to the smartreflex driver.
 
-config OMAP_SMARTREFLEX_CLASS3
+config POWER_AVS_OMAP_CLASS3
 	bool "Class 3 mode of Smartreflex Implementation"
-	depends on OMAP_SMARTREFLEX && TWL4030_CORE
+	depends on POWER_AVS_OMAP && TWL4030_CORE
 	help
 	  Say Y to enable Class 3 implementation of Smartreflex
 
 	  Class 3 implementation of Smartreflex employs continuous hardware
 	  voltage calibration.
 
+endif # POWER_AVS
+
 config OMAP_RESET_CLOCKS
 	bool "Reset unused clocks during boot"
 	depends on ARCH_OMAP
diff --git a/include/linux/power/smartreflex.h b/include/linux/power/smartreflex.h
index 222f901..3101e62 100644
--- a/include/linux/power/smartreflex.h
+++ b/include/linux/power/smartreflex.h
@@ -207,7 +207,7 @@ struct omap_smartreflex_dev_attr {
 	const char      *sensor_voltdm_name;
 };
 
-#ifdef CONFIG_OMAP_SMARTREFLEX
+#ifdef CONFIG_POWER_AVS_OMAP
 /*
  * The smart reflex driver supports CLASS1 CLASS2 and CLASS3 SR.
  * The smartreflex class driver should pass the class type.
-- 
1.7.5.4


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

* [PATCH V3 10/10] ARM: OMAP: SmartReflex: Move smartreflex driver to drivers/
  2012-04-26 17:40 [PATCH V3 00/10] PM: Create the AVS(Adaptive Voltage Scaling) Keerthy
                   ` (8 preceding siblings ...)
  2012-04-26 17:40 ` [PATCH V3 09/10] ARM: OMAP2+: SmartReflex: add POWER_AVS Kconfig options Keerthy
@ 2012-04-26 17:40 ` Keerthy
  2012-04-26 19:11 ` [PATCH V3 00/10] PM: Create the AVS(Adaptive Voltage Scaling) Mark Brown
  2012-05-07 23:51 ` Kevin Hilman
  11 siblings, 0 replies; 49+ messages in thread
From: Keerthy @ 2012-04-26 17:40 UTC (permalink / raw)
  To: linux-omap, linux-arm-kernel, khilman, rjw, linux-kernel, linux-pm
  Cc: j-pihet, j-keerthy

From: Jean Pihet <j-pihet@ti.com> 

After a clean-up of the interfaces the OMAP Smartreflex IP driver is now a
generic driver. Move it to drivers/power/avs/.

The build is controlled by the following Kconfig options:
 . CONFIG_POWER_AVS: general knob for Adaptive Voltage Scaling support,
 . CONFIG_POWER_AVS_OMAP: AVS(Adaptive Voltage Scaling)
   support on OMAP containing the version 1 or version 2 of the SmartReflex IP,
 . CONFIG_POWER_AVS_OMAP_CLASS3: Class 3 implementation of Smartreflex.

Signed-off-by: Jean Pihet <j-pihet@ti.com>
Signed-off-by: J Keerthy <j-keerthy@ti.com>
---
 arch/arm/mach-omap2/Makefile                       |    4 +-
 arch/arm/plat-omap/Kconfig                         |   22 ++-----------------
 drivers/power/Kconfig                              |    2 +
 drivers/power/Makefile                             |    1 +
 drivers/power/avs/Kconfig                          |   12 ++++++++++
 drivers/power/avs/Makefile                         |    1 +
 .../mach-omap2 => drivers/power/avs}/smartreflex.c |    5 +---
 7 files changed, 22 insertions(+), 25 deletions(-)
 create mode 100644 drivers/power/avs/Kconfig
 create mode 100644 drivers/power/avs/Makefile
 rename {arch/arm/mach-omap2 => drivers/power/avs}/smartreflex.c (99%)

diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
index 85ca797..43ffde4 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -70,8 +70,8 @@ obj-$(CONFIG_ARCH_OMAP4)		+= pm44xx.o omap-mpuss-lowpower.o \
 					   cpuidle44xx.o
 obj-$(CONFIG_PM_DEBUG)			+= pm-debug.o
 
-obj-$(CONFIG_POWER_AVS_OMAP)		+= sr_device.o smartreflex.o
-obj-$(CONFIG_POWER_AVS_OMAP_CLASS3)	+= smartreflex-class3.o
+obj-$(CONFIG_POWER_AVS_OMAP)		+= sr_device.o
+obj-$(CONFIG_POWER_AVS_OMAP_CLASS3)    += smartreflex-class3.o
 
 AFLAGS_sleep24xx.o			:=-Wa,-march=armv6
 AFLAGS_sleep34xx.o			:=-Wa,-march=armv7-a$(plus_sec)
diff --git a/arch/arm/plat-omap/Kconfig b/arch/arm/plat-omap/Kconfig
index bba384d..816dec0 100644
--- a/arch/arm/plat-omap/Kconfig
+++ b/arch/arm/plat-omap/Kconfig
@@ -45,26 +45,12 @@ config OMAP_DEBUG_LEDS
 	depends on OMAP_DEBUG_DEVICES
 	default y if LEDS_CLASS
 
-menuconfig POWER_AVS
-	tristate "Adaptive Voltage Scaling class support"
-	help
-	  AVS(Adaptive Voltage Scaling) is a power management technique which
-	  finely controls the operating voltage of a device in order to optimize
-	   (i.e. reduce) its power consumption.
-	  At a given operating point the voltage is adapted depending on
-	  static factors (chip manufacturing process) and dynamic factors
-	  (temperature depending performance).
-	  AVS is also called SmartReflex on OMAP devices.
-
-	  Say Y here to enable Adaptive Voltage Scaling class support.
-
-if POWER_AVS
-
 config POWER_AVS_OMAP
 	bool "AVS(Adaptive Voltage Scaling) support for OMAP IP versions 1&2"
-	depends on (ARCH_OMAP3 || ARCH_OMAP4) && PM
+	depends on POWER_AVS && (ARCH_OMAP3 || ARCH_OMAP4) && PM
 	help
-	  Say Y to enable AVS support on OMAP containing the version 1 or
+	  Say Y to enable AVS(Adaptive Voltage Scaling)
+	  support on OMAP containing the version 1 or
 	  version 2 of the SmartReflex IP.
 	  V1 is the 65nm version used in OMAP3430.
 	  V2 is the update for the 45nm version of the IP used in OMAP3630
@@ -89,8 +75,6 @@ config POWER_AVS_OMAP_CLASS3
 	  Class 3 implementation of Smartreflex employs continuous hardware
 	  voltage calibration.
 
-endif # POWER_AVS
-
 config OMAP_RESET_CLOCKS
 	bool "Reset unused clocks during boot"
 	depends on ARCH_OMAP
diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index 99dc29f..d416773 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -308,3 +308,5 @@ config AB8500_BATTERY_THERM_ON_BATCTRL
 	  Say Y to enable battery temperature measurements using
 	  thermistor connected on BATCTRL ADC.
 endif # POWER_SUPPLY
+
+source "drivers/power/avs/Kconfig"
diff --git a/drivers/power/Makefile b/drivers/power/Makefile
index b6b2434..ee58afb 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/Makefile
@@ -43,4 +43,5 @@ obj-$(CONFIG_CHARGER_GPIO)	+= gpio-charger.o
 obj-$(CONFIG_CHARGER_MANAGER)	+= charger-manager.o
 obj-$(CONFIG_CHARGER_MAX8997)	+= max8997_charger.o
 obj-$(CONFIG_CHARGER_MAX8998)	+= max8998_charger.o
+obj-$(CONFIG_POWER_AVS)		+= avs/
 obj-$(CONFIG_CHARGER_SMB347)	+= smb347-charger.o
diff --git a/drivers/power/avs/Kconfig b/drivers/power/avs/Kconfig
new file mode 100644
index 0000000..18493f7
--- /dev/null
+++ b/drivers/power/avs/Kconfig
@@ -0,0 +1,12 @@
+menuconfig POWER_AVS
+	tristate "Adaptive Voltage Scaling class support"
+	help
+	  AVS is a power management technique which finely controls the
+	  operating voltage of a device in order to optimize (i.e. reduce)
+	  its power consumption.
+	  At a given operating point the voltage is adapted depending on
+	  static factors (chip manufacturing process) and dynamic factors
+	  (temperature depending performance).
+	  AVS is also called SmartReflex on OMAP devices.
+
+	  Say Y here to enable Adaptive Voltage Scaling class support.
diff --git a/drivers/power/avs/Makefile b/drivers/power/avs/Makefile
new file mode 100644
index 0000000..0843386
--- /dev/null
+++ b/drivers/power/avs/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_POWER_AVS_OMAP)		+= smartreflex.o
diff --git a/arch/arm/mach-omap2/smartreflex.c b/drivers/power/avs/smartreflex.c
similarity index 99%
rename from arch/arm/mach-omap2/smartreflex.c
rename to drivers/power/avs/smartreflex.c
index 515041c..44efc6e 100644
--- a/arch/arm/mach-omap2/smartreflex.c
+++ b/drivers/power/avs/smartreflex.c
@@ -3,7 +3,7 @@
  *
  * Author: Thara Gopinath	<thara@ti.com>
  *
- * Copyright (C) 2010 Texas Instruments, Inc.
+ * Copyright (C) 2012 Texas Instruments, Inc.
  * Thara Gopinath <thara@ti.com>
  *
  * Copyright (C) 2008 Nokia Corporation
@@ -27,9 +27,6 @@
 #include <linux/pm_runtime.h>
 #include <linux/power/smartreflex.h>
 
-#include "common.h"
-#include "pm.h"
-
 #define SMARTREFLEX_NAME_LEN	16
 #define NVALUE_NAME_LEN		40
 #define SR_DISABLE_TIMEOUT	200
-- 
1.7.5.4


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

* Re: [PATCH V3 00/10] PM: Create the AVS(Adaptive Voltage Scaling)
  2012-04-26 17:40 [PATCH V3 00/10] PM: Create the AVS(Adaptive Voltage Scaling) Keerthy
                   ` (9 preceding siblings ...)
  2012-04-26 17:40 ` [PATCH V3 10/10] ARM: OMAP: SmartReflex: Move smartreflex driver to drivers/ Keerthy
@ 2012-04-26 19:11 ` Mark Brown
  2012-04-27  5:39   ` J, KEERTHY
  2012-05-07 23:51 ` Kevin Hilman
  11 siblings, 1 reply; 49+ messages in thread
From: Mark Brown @ 2012-04-26 19:11 UTC (permalink / raw)
  To: Keerthy
  Cc: linux-omap, linux-arm-kernel, khilman, rjw, linux-kernel,
	linux-pm, j-pihet

On Thu, Apr 26, 2012 at 11:10:31PM +0530, Keerthy wrote:
> From: J Keerthy <j-keerthy@ti.com>
> 
> AVS(Adaptive Voltage Scaling) is a power management technique which
> controls the operating voltage of a device in order to optimize (i.e. reduce)
> its power consumption. The voltage is adapted depending on static factors
> (chip manufacturing process) and dynamic factors (temperature
> depending performance).
> The TI AVS solution is named Smartreflex. 

What's the relationship between this and existing things like devfreq
and cpufreq?  It'd be better if the changelogs made this clear and
provided an overview of how all these different subsystems are intended
to fit together.

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

* Re: [PATCH V3 00/10] PM: Create the AVS(Adaptive Voltage Scaling)
  2012-04-26 19:11 ` [PATCH V3 00/10] PM: Create the AVS(Adaptive Voltage Scaling) Mark Brown
@ 2012-04-27  5:39   ` J, KEERTHY
  2012-04-27 17:56     ` Mark Brown
  0 siblings, 1 reply; 49+ messages in thread
From: J, KEERTHY @ 2012-04-27  5:39 UTC (permalink / raw)
  To: Mark Brown
  Cc: linux-omap, linux-arm-kernel, khilman, rjw, linux-kernel,
	linux-pm, j-pihet

Hello Mark,

On Fri, Apr 27, 2012 at 12:41 AM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Thu, Apr 26, 2012 at 11:10:31PM +0530, Keerthy wrote:
>> From: J Keerthy <j-keerthy@ti.com>
>>
>> AVS(Adaptive Voltage Scaling) is a power management technique which
>> controls the operating voltage of a device in order to optimize (i.e. reduce)
>> its power consumption. The voltage is adapted depending on static factors
>> (chip manufacturing process) and dynamic factors (temperature
>> depending performance).
>> The TI AVS solution is named Smartreflex.
>
> What's the relationship between this and existing things like devfreq
> and cpufreq?  It'd be better if the changelogs made this clear and
> provided an overview of how all these different subsystems are intended
> to fit together.

Devfreq and cpufreq are related to dynamic frequency/voltage switching between
pre defined Operating Performance Points or the OPPs. Every OPP being
a voltage/frequency pair. Smartreflex is a different
power management technique.

SmartReflex is a technology that uses adaptive
power supply to achieve the goal of reducing active power consumption.
With SmartReflex, the power supply voltage can be adapted to the silicon
performance either statically (for example, adapted to the manufacturing process
of a given device), or dynamically (for example, adapted to the temperature
induced current performance of the device).

So for every OPP(voltage/frequency pair) depending on the silicon process and
temperature the Smartreflex tries to get the voltage to an optimal
value at which
the corresponding frequency can be sustained.

-- 
Regards and Thanks,
Keerthy

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

* Re: [PATCH V3 00/10] PM: Create the AVS(Adaptive Voltage Scaling)
  2012-04-27  5:39   ` J, KEERTHY
@ 2012-04-27 17:56     ` Mark Brown
  2012-04-27 21:01       ` Kevin Hilman
  0 siblings, 1 reply; 49+ messages in thread
From: Mark Brown @ 2012-04-27 17:56 UTC (permalink / raw)
  To: J, KEERTHY
  Cc: linux-omap, linux-arm-kernel, khilman, rjw, linux-kernel,
	linux-pm, j-pihet

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

On Fri, Apr 27, 2012 at 11:09:10AM +0530, J, KEERTHY wrote:

> Devfreq and cpufreq are related to dynamic frequency/voltage switching between
> pre defined Operating Performance Points or the OPPs. Every OPP being
> a voltage/frequency pair. Smartreflex is a different
> power management technique.

But presumably these things should integrate somehow - for example,
should devfreq and cpufreq be providing inputs into what AVS is doing,
and if so how?

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

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

* Re: [PATCH V3 00/10] PM: Create the AVS(Adaptive Voltage Scaling)
  2012-04-27 17:56     ` Mark Brown
@ 2012-04-27 21:01       ` Kevin Hilman
  2012-04-30  4:25         ` J, KEERTHY
                           ` (2 more replies)
  0 siblings, 3 replies; 49+ messages in thread
From: Kevin Hilman @ 2012-04-27 21:01 UTC (permalink / raw)
  To: Mark Brown
  Cc: J, KEERTHY, linux-omap, linux-arm-kernel, rjw, linux-kernel,
	linux-pm, j-pihet

Hi Mark,

Mark Brown <broonie@opensource.wolfsonmicro.com> writes:

> On Fri, Apr 27, 2012 at 11:09:10AM +0530, J, KEERTHY wrote:
>
>> Devfreq and cpufreq are related to dynamic frequency/voltage switching between
>> pre defined Operating Performance Points or the OPPs. Every OPP being
>> a voltage/frequency pair. Smartreflex is a different
>> power management technique.
>
> But presumably these things should integrate somehow - for example,
> should devfreq and cpufreq be providing inputs into what AVS is doing,
> and if so how?

The way it is currently designed, cpufreq/devfreq/regulator layers don't
need to know about AVS.

The higher-level layers only know about the "nominal" voltage.  AVS
hardware does automatic, adaptive, micro-adjustments around that nominal
voltage, and these micro-adjustments are managed by the AVS hardware
sending commands to the PMIC.  (specifically, on OMAP, the AVS sensors
provide inputs to the voltage processor (VP) which provide inputs to the
voltage controller (VC) which sends commands to the PMIC[1].)

The driver proposed here is primarily for initializing the various
parameters/sensitivity/etc. of the AVS hardware, but the actual voltage
adjustments are done in hardware by VC/VP.

The only thing the higher-level layers might potentially need to do to
enable/disable AVS around transitions (e.g. when changing OPP, AVS is
disabled before changing OPP and only re-enabled when the new nominal
voltage has been acheived.)

On OMAP, we handle this inside the OMAP-specific voltage layer which is
called by the regulator framework, so even the regulators do not need
any knowledge of AVS.

Kevin

[1] Figure 3-76 in OMAP4430 ES2.1 Public TRM vAD provides a 
    detailed diagram:
    http://www.ti.com/pdfs/wtbu/OMAP4430_ES2.x_PUBLIC_TRM_vAD.zip

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

* Re: [PATCH V3 00/10] PM: Create the AVS(Adaptive Voltage Scaling)
  2012-04-27 21:01       ` Kevin Hilman
@ 2012-04-30  4:25         ` J, KEERTHY
  2012-04-30  9:54         ` Mark Brown
  2012-05-04  8:21         ` AnilKumar, Chimata
  2 siblings, 0 replies; 49+ messages in thread
From: J, KEERTHY @ 2012-04-30  4:25 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: Mark Brown, linux-omap, linux-arm-kernel, rjw, linux-kernel,
	linux-pm, j-pihet

On Sat, Apr 28, 2012 at 2:31 AM, Kevin Hilman <khilman@ti.com> wrote:
> Hi Mark,
>
> Mark Brown <broonie@opensource.wolfsonmicro.com> writes:
>
>> On Fri, Apr 27, 2012 at 11:09:10AM +0530, J, KEERTHY wrote:
>>
>>> Devfreq and cpufreq are related to dynamic frequency/voltage switching between
>>> pre defined Operating Performance Points or the OPPs. Every OPP being
>>> a voltage/frequency pair. Smartreflex is a different
>>> power management technique.
>>
>> But presumably these things should integrate somehow - for example,
>> should devfreq and cpufreq be providing inputs into what AVS is doing,
>> and if so how?
>
> The way it is currently designed, cpufreq/devfreq/regulator layers don't
> need to know about AVS.
>
> The higher-level layers only know about the "nominal" voltage.  AVS
> hardware does automatic, adaptive, micro-adjustments around that nominal
> voltage, and these micro-adjustments are managed by the AVS hardware
> sending commands to the PMIC.  (specifically, on OMAP, the AVS sensors
> provide inputs to the voltage processor (VP) which provide inputs to the
> voltage controller (VC) which sends commands to the PMIC[1].)
>
> The driver proposed here is primarily for initializing the various
> parameters/sensitivity/etc. of the AVS hardware, but the actual voltage
> adjustments are done in hardware by VC/VP.
>
> The only thing the higher-level layers might potentially need to do to
> enable/disable AVS around transitions (e.g. when changing OPP, AVS is
> disabled before changing OPP and only re-enabled when the new nominal
> voltage has been acheived.)
>
> On OMAP, we handle this inside the OMAP-specific voltage layer which is
> called by the regulator framework, so even the regulators do not need
> any knowledge of AVS.
>
> Kevin
>
> [1] Figure 3-76 in OMAP4430 ES2.1 Public TRM vAD provides a
>    detailed diagram:
>    http://www.ti.com/pdfs/wtbu/OMAP4430_ES2.x_PUBLIC_TRM_vAD.zip

Thanks for the detailed answer Kevin.

-- 
Regards and Thanks,
Keerthy

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

* Re: [PATCH V3 00/10] PM: Create the AVS(Adaptive Voltage Scaling)
  2012-04-27 21:01       ` Kevin Hilman
  2012-04-30  4:25         ` J, KEERTHY
@ 2012-04-30  9:54         ` Mark Brown
  2012-04-30 21:51           ` Kevin Hilman
  2012-05-04  8:21         ` AnilKumar, Chimata
  2 siblings, 1 reply; 49+ messages in thread
From: Mark Brown @ 2012-04-30  9:54 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: J, KEERTHY, linux-omap, linux-arm-kernel, rjw, linux-kernel,
	linux-pm, j-pihet, durgadoss.r

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

On Fri, Apr 27, 2012 at 02:01:17PM -0700, Kevin Hilman wrote:
> Mark Brown <broonie@opensource.wolfsonmicro.com> writes:

> > But presumably these things should integrate somehow - for example,
> > should devfreq and cpufreq be providing inputs into what AVS is doing,
> > and if so how?

> The way it is currently designed, cpufreq/devfreq/regulator layers don't
> need to know about AVS.

Yes, and that was a part of my concern, but see below.

> The higher-level layers only know about the "nominal" voltage.  AVS
> hardware does automatic, adaptive, micro-adjustments around that nominal
> voltage, and these micro-adjustments are managed by the AVS hardware
> sending commands to the PMIC.  (specifically, on OMAP, the AVS sensors
> provide inputs to the voltage processor (VP) which provide inputs to the
> voltage controller (VC) which sends commands to the PMIC[1].)

Right, that's what I'd understood it to be.  

> The driver proposed here is primarily for initializing the various
> parameters/sensitivity/etc. of the AVS hardware, but the actual voltage
> adjustments are done in hardware by VC/VP.

It's not just a driver, though - it's also creating this power/avs
thing, though now I look at the code rather than just its shape there's
not actually an abstraction being added here, it's mostly just straight
code motion of the arch/arm code that's there already.  The changelog
and the shape of the code make it sound like this is intended to be
somewhat generic when really it's providing some OMAP specific tuning
for the device which is much less of a concern.

I guess for now it's probably OK to just clarify in the documentation
and say that whoever adds the second driver has to work on making this
generic :)

This does also sound rather like it's in a similar area to the current
management work which Durgadoss R (CCed) was working on, though with a
slightly different application and in the OMAP case it's pretty much all
hidden in the external processor.

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

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

* Re: [PATCH V3 00/10] PM: Create the AVS(Adaptive Voltage Scaling)
  2012-04-30  9:54         ` Mark Brown
@ 2012-04-30 21:51           ` Kevin Hilman
  2012-05-02  5:04             ` J, KEERTHY
  0 siblings, 1 reply; 49+ messages in thread
From: Kevin Hilman @ 2012-04-30 21:51 UTC (permalink / raw)
  To: Mark Brown
  Cc: J, KEERTHY, linux-omap, linux-arm-kernel, rjw, linux-kernel,
	linux-pm, j-pihet, durgadoss.r

Mark Brown <broonie@opensource.wolfsonmicro.com> writes:

> On Fri, Apr 27, 2012 at 02:01:17PM -0700, Kevin Hilman wrote:
>> Mark Brown <broonie@opensource.wolfsonmicro.com> writes:
>
>> > But presumably these things should integrate somehow - for example,
>> > should devfreq and cpufreq be providing inputs into what AVS is doing,
>> > and if so how?
>
>> The way it is currently designed, cpufreq/devfreq/regulator layers don't
>> need to know about AVS.
>
> Yes, and that was a part of my concern, but see below.
>
>> The higher-level layers only know about the "nominal" voltage.  AVS
>> hardware does automatic, adaptive, micro-adjustments around that nominal
>> voltage, and these micro-adjustments are managed by the AVS hardware
>> sending commands to the PMIC.  (specifically, on OMAP, the AVS sensors
>> provide inputs to the voltage processor (VP) which provide inputs to the
>> voltage controller (VC) which sends commands to the PMIC[1].)
>
> Right, that's what I'd understood it to be.  
>
>> The driver proposed here is primarily for initializing the various
>> parameters/sensitivity/etc. of the AVS hardware, but the actual voltage
>> adjustments are done in hardware by VC/VP.
>
> It's not just a driver, though - it's also creating this power/avs
> thing, though now I look at the code rather than just its shape there's
> not actually an abstraction being added here, it's mostly just straight
> code motion of the arch/arm code that's there already.  The changelog
> and the shape of the code make it sound like this is intended to be
> somewhat generic when really it's providing some OMAP specific tuning
> for the device which is much less of a concern.
>
> I guess for now it's probably OK to just clarify in the documentation
> and say that whoever adds the second driver has to work on making this
> generic :)

Agreed.

In a different thread (which I can't seem to find now) we discussed this
as well, so it just sounds like the changelog should clarify this a bit
better.

Kevin

> This does also sound rather like it's in a similar area to the current
> management work which Durgadoss R (CCed) was working on, though with a
> slightly different application and in the OMAP case it's pretty much all
> hidden in the external processor.


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

* Re: [PATCH V3 00/10] PM: Create the AVS(Adaptive Voltage Scaling)
  2012-04-30 21:51           ` Kevin Hilman
@ 2012-05-02  5:04             ` J, KEERTHY
  2012-05-04  5:05               ` J, KEERTHY
  0 siblings, 1 reply; 49+ messages in thread
From: J, KEERTHY @ 2012-05-02  5:04 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: Mark Brown, linux-omap, linux-arm-kernel, rjw, linux-kernel,
	linux-pm, j-pihet, durgadoss.r

On Tue, May 1, 2012 at 3:21 AM, Kevin Hilman <khilman@ti.com> wrote:
> Mark Brown <broonie@opensource.wolfsonmicro.com> writes:
>
>> On Fri, Apr 27, 2012 at 02:01:17PM -0700, Kevin Hilman wrote:
>>> Mark Brown <broonie@opensource.wolfsonmicro.com> writes:
>>
>>> > But presumably these things should integrate somehow - for example,
>>> > should devfreq and cpufreq be providing inputs into what AVS is doing,
>>> > and if so how?
>>
>>> The way it is currently designed, cpufreq/devfreq/regulator layers don't
>>> need to know about AVS.
>>
>> Yes, and that was a part of my concern, but see below.
>>
>>> The higher-level layers only know about the "nominal" voltage.  AVS
>>> hardware does automatic, adaptive, micro-adjustments around that nominal
>>> voltage, and these micro-adjustments are managed by the AVS hardware
>>> sending commands to the PMIC.  (specifically, on OMAP, the AVS sensors
>>> provide inputs to the voltage processor (VP) which provide inputs to the
>>> voltage controller (VC) which sends commands to the PMIC[1].)
>>
>> Right, that's what I'd understood it to be.
>>
>>> The driver proposed here is primarily for initializing the various
>>> parameters/sensitivity/etc. of the AVS hardware, but the actual voltage
>>> adjustments are done in hardware by VC/VP.
>>
>> It's not just a driver, though - it's also creating this power/avs
>> thing, though now I look at the code rather than just its shape there's
>> not actually an abstraction being added here, it's mostly just straight
>> code motion of the arch/arm code that's there already.  The changelog
>> and the shape of the code make it sound like this is intended to be
>> somewhat generic when really it's providing some OMAP specific tuning
>> for the device which is much less of a concern.
>>
>> I guess for now it's probably OK to just clarify in the documentation
>> and say that whoever adds the second driver has to work on making this
>> generic :)
>
> Agreed.
>
> In a different thread (which I can't seem to find now) we discussed this
> as well, so it just sounds like the changelog should clarify this a bit
> better.

Kevin/Mark,

Thanks for the feedback. I will add more documentation
to clarify this aspect. Please let me know if there are any more
things to be taken care of in this patch set.

>
> Kevin
>
>> This does also sound rather like it's in a similar area to the current
>> management work which Durgadoss R (CCed) was working on, though with a
>> slightly different application and in the OMAP case it's pretty much all
>> hidden in the external processor.
>



-- 
Regards and Thanks,
Keerthy

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

* Re: [PATCH V3 00/10] PM: Create the AVS(Adaptive Voltage Scaling)
  2012-05-02  5:04             ` J, KEERTHY
@ 2012-05-04  5:05               ` J, KEERTHY
  0 siblings, 0 replies; 49+ messages in thread
From: J, KEERTHY @ 2012-05-04  5:05 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: Mark Brown, linux-omap, linux-arm-kernel, rjw, linux-kernel,
	linux-pm, j-pihet, durgadoss.r

On Wed, May 2, 2012 at 10:34 AM, J, KEERTHY <j-keerthy@ti.com> wrote:
> On Tue, May 1, 2012 at 3:21 AM, Kevin Hilman <khilman@ti.com> wrote:
>> Mark Brown <broonie@opensource.wolfsonmicro.com> writes:
>>
>>> On Fri, Apr 27, 2012 at 02:01:17PM -0700, Kevin Hilman wrote:
>>>> Mark Brown <broonie@opensource.wolfsonmicro.com> writes:
>>>
>>>> > But presumably these things should integrate somehow - for example,
>>>> > should devfreq and cpufreq be providing inputs into what AVS is doing,
>>>> > and if so how?
>>>
>>>> The way it is currently designed, cpufreq/devfreq/regulator layers don't
>>>> need to know about AVS.
>>>
>>> Yes, and that was a part of my concern, but see below.
>>>
>>>> The higher-level layers only know about the "nominal" voltage.  AVS
>>>> hardware does automatic, adaptive, micro-adjustments around that nominal
>>>> voltage, and these micro-adjustments are managed by the AVS hardware
>>>> sending commands to the PMIC.  (specifically, on OMAP, the AVS sensors
>>>> provide inputs to the voltage processor (VP) which provide inputs to the
>>>> voltage controller (VC) which sends commands to the PMIC[1].)
>>>
>>> Right, that's what I'd understood it to be.
>>>
>>>> The driver proposed here is primarily for initializing the various
>>>> parameters/sensitivity/etc. of the AVS hardware, but the actual voltage
>>>> adjustments are done in hardware by VC/VP.
>>>
>>> It's not just a driver, though - it's also creating this power/avs
>>> thing, though now I look at the code rather than just its shape there's
>>> not actually an abstraction being added here, it's mostly just straight
>>> code motion of the arch/arm code that's there already.  The changelog
>>> and the shape of the code make it sound like this is intended to be
>>> somewhat generic when really it's providing some OMAP specific tuning
>>> for the device which is much less of a concern.
>>>
>>> I guess for now it's probably OK to just clarify in the documentation
>>> and say that whoever adds the second driver has to work on making this
>>> generic :)
>>
>> Agreed.
>>
>> In a different thread (which I can't seem to find now) we discussed this
>> as well, so it just sounds like the changelog should clarify this a bit
>> better.
>
> Kevin/Mark,
>
> Thanks for the feedback. I will add more documentation
> to clarify this aspect. Please let me know if there are any more
> things to be taken care of in this patch set.

Hello Kevin,

A gentle ping on this series. Any comments on this?

>
>>
>> Kevin
>>
>>> This does also sound rather like it's in a similar area to the current
>>> management work which Durgadoss R (CCed) was working on, though with a
>>> slightly different application and in the OMAP case it's pretty much all
>>> hidden in the external processor.
>>
>
>
>
> --
> Regards and Thanks,
> Keerthy



-- 
Regards and Thanks,
Keerthy

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

* RE: [PATCH V3 00/10] PM: Create the AVS(Adaptive Voltage Scaling)
  2012-04-27 21:01       ` Kevin Hilman
  2012-04-30  4:25         ` J, KEERTHY
  2012-04-30  9:54         ` Mark Brown
@ 2012-05-04  8:21         ` AnilKumar, Chimata
  2012-05-07 23:48           ` Kevin Hilman
  2 siblings, 1 reply; 49+ messages in thread
From: AnilKumar, Chimata @ 2012-05-04  8:21 UTC (permalink / raw)
  To: Hilman, Kevin, Mark Brown
  Cc: J, KEERTHY, linux-omap, linux-arm-kernel, rjw, linux-kernel,
	linux-pm, Pihet-XID, Jean

On Sat, Apr 28, 2012 at 02:31:17, Hilman, Kevin wrote:
> Hi Mark,
> 
> Mark Brown <broonie@opensource.wolfsonmicro.com> writes:
> 
> > On Fri, Apr 27, 2012 at 11:09:10AM +0530, J, KEERTHY wrote:
> >
> >> Devfreq and cpufreq are related to dynamic frequency/voltage switching between
> >> pre defined Operating Performance Points or the OPPs. Every OPP being
> >> a voltage/frequency pair. Smartreflex is a different
> >> power management technique.
> >
> > But presumably these things should integrate somehow - for example,
> > should devfreq and cpufreq be providing inputs into what AVS is doing,
> > and if so how?
> 
> The way it is currently designed, cpufreq/devfreq/regulator layers don't
> need to know about AVS.
> 
> The higher-level layers only know about the "nominal" voltage.  AVS
> hardware does automatic, adaptive, micro-adjustments around that nominal
> voltage, and these micro-adjustments are managed by the AVS hardware
> sending commands to the PMIC.  (specifically, on OMAP, the AVS sensors
> provide inputs to the voltage processor (VP) which provide inputs to the
> voltage controller (VC) which sends commands to the PMIC[1].)
> 
> The driver proposed here is primarily for initializing the various
> parameters/sensitivity/etc. of the AVS hardware, but the actual voltage
> adjustments are done in hardware by VC/VP.
> 
> The only thing the higher-level layers might potentially need to do to
> enable/disable AVS around transitions (e.g. when changing OPP, AVS is
> disabled before changing OPP and only re-enabled when the new nominal
> voltage has been acheived.)
> 
> On OMAP, we handle this inside the OMAP-specific voltage layer which is
> called by the regulator framework, so even the regulators do not need
> any knowledge of AVS.

Kevin,

I want to point out some cases of SR implementation where this may not
be true.

Devices like DM8168, DM8148 and AM335X use Class 2B implementation of SR.

Under this, SR module issues an interrupt to ARM when there is a need to
change the voltage based on temperature changes, ageing etc.

Once the interrupt arrives, kernel needs to adjust voltage using regulator API.
The voltage change is a micro adjustment as in other SR classes.

The SR class 2B implementation on these devices does not exist in mainline.
I can point to some public repositories if you are interested in taking a look at
the current code.

Implementation of this SR method is must on at least the DM8168 device and
I know some customers who are using it on their production systems.

Regards
AnilKumar

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

* RE: [PATCH V3 04/10] ARM: OMAP3: hwmod: rename the smartreflex entries
  2012-04-26 17:40 ` [PATCH V3 04/10] ARM: OMAP3: hwmod: rename the smartreflex entries Keerthy
@ 2012-05-04  8:30   ` AnilKumar, Chimata
  2012-05-04 10:11     ` J, KEERTHY
  0 siblings, 1 reply; 49+ messages in thread
From: AnilKumar, Chimata @ 2012-05-04  8:30 UTC (permalink / raw)
  To: J, KEERTHY, linux-omap, linux-arm-kernel, Hilman, Kevin, rjw,
	linux-kernel, linux-pm
  Cc: Pihet-XID, Jean

On Thu, Apr 26, 2012 at 23:10:35, J, KEERTHY wrote:
> From: Jean Pihet <j-pihet@ti.com>
> 
> Change the name field value to better reflect the smartreflex
> integration in the system.
> 
> Signed-off-by: Jean Pihet <j-pihet@ti.com>
> Signed-off-by: J Keerthy <j-keerthy@ti.com>
> ---
>  arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |    8 ++++----
>  arch/arm/mach-omap2/smartreflex.c          |    2 +-
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
> index 144d118..15907b0 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
> @@ -1324,7 +1324,7 @@ static struct omap_hwmod_irq_info omap3_smartreflex_mpu_irqs[] = {
>  };
>  
>  static struct omap_hwmod omap34xx_sr1_hwmod = {
> -	.name		= "sr1",
> +	.name		= "smartreflex_mpu_iva",
>  	.class		= &omap34xx_smartreflex_hwmod_class,
>  	.main_clk	= "sr1_fck",
>  	.prcm		= {
> @@ -1342,7 +1342,7 @@ static struct omap_hwmod omap34xx_sr1_hwmod = {
>  };
>  
>  static struct omap_hwmod omap36xx_sr1_hwmod = {
> -	.name		= "sr1",
> +	.name		= "smartreflex_mpu_iva",
>  	.class		= &omap36xx_smartreflex_hwmod_class,
>  	.main_clk	= "sr1_fck",
>  	.prcm		= {
> @@ -1369,7 +1369,7 @@ static struct omap_hwmod_irq_info omap3_smartreflex_core_irqs[] = {
>  };
>  
>  static struct omap_hwmod omap34xx_sr2_hwmod = {
> -	.name		= "sr2",
> +	.name		= "smartreflex_core",
>  	.class		= &omap34xx_smartreflex_hwmod_class,
>  	.main_clk	= "sr2_fck",
>  	.prcm		= {
> @@ -1387,7 +1387,7 @@ static struct omap_hwmod omap34xx_sr2_hwmod = {
>  };
>  
>  static struct omap_hwmod omap36xx_sr2_hwmod = {
> -	.name		= "sr2",
> +	.name		= "smartreflex_core",
>  	.class		= &omap36xx_smartreflex_hwmod_class,
>  	.main_clk	= "sr2_fck",
>  	.prcm		= {
> diff --git a/arch/arm/mach-omap2/smartreflex.c b/arch/arm/mach-omap2/smartreflex.c
> index 2edd1e2..d859277 100644
> --- a/arch/arm/mach-omap2/smartreflex.c
> +++ b/arch/arm/mach-omap2/smartreflex.c
> @@ -183,7 +183,7 @@ static void sr_set_regfields(struct omap_sr *sr)
>  		sr->err_weight = OMAP3430_SR_ERRWEIGHT;
>  		sr->err_maxlimit = OMAP3430_SR_ERRMAXLIMIT;
>  		sr->accum_data = OMAP3430_SR_ACCUMDATA;
> -		if (!(strcmp(sr->name, "sr1"))) {
> +		if (!(strcmp(sr->name, "smartreflex_mpu_iva"))) {

What if voltage rail is different for mpu and iva? I have seen some devices
supports SmartReflex have different voltage rails for mpu and iva.

Regards
AnilKumar

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

* RE: [PATCH V3 05/10] ARM: OMAP2+: SmartReflex: introduce a busy loop condition test macro
  2012-04-26 17:40 ` [PATCH V3 05/10] ARM: OMAP2+: SmartReflex: introduce a busy loop condition test macro Keerthy
@ 2012-05-04  9:12   ` AnilKumar, Chimata
  2012-05-07  5:21     ` J, KEERTHY
  0 siblings, 1 reply; 49+ messages in thread
From: AnilKumar, Chimata @ 2012-05-04  9:12 UTC (permalink / raw)
  To: J, KEERTHY, linux-omap, linux-arm-kernel, Hilman, Kevin, rjw,
	linux-kernel, linux-pm
  Cc: Pihet-XID, Jean

On Thu, Apr 26, 2012 at 23:10:36, J, KEERTHY wrote:
> From: Jean Pihet <j-pihet@ti.com>
> 
> Now that omap_test_timeout is only accessible from mach-omap2/,
> introduce a similar function for SR.
> 
> This change makes the SmartReflex implementation ready for the move
> to drivers/.
> 
> Signed-off-by: Jean Pihet <j-pihet@ti.com>
> Signed-off-by: J Keerthy <j-keerthy@ti.com>
> ---
>  arch/arm/mach-omap2/smartreflex.c |   12 ++++++------
>  include/linux/power/smartreflex.h |   23 ++++++++++++++++++++++-
>  2 files changed, 28 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/smartreflex.c b/arch/arm/mach-omap2/smartreflex.c
> index d859277..acef08d 100644
> --- a/arch/arm/mach-omap2/smartreflex.c
> +++ b/arch/arm/mach-omap2/smartreflex.c
> @@ -289,9 +289,9 @@ static void sr_v1_disable(struct omap_sr *sr)
>  	 * Wait for SR to be disabled.
>  	 * wait until ERRCONFIG.MCUDISACKINTST = 1. Typical latency is 1us.
>  	 */
> -	omap_test_timeout((sr_read_reg(sr, ERRCONFIG_V1) &
> -			ERRCONFIG_MCUDISACKINTST), SR_DISABLE_TIMEOUT,
> -			timeout);
> +	sr_test_cond_timeout((sr_read_reg(sr, ERRCONFIG_V1) &
> +			     ERRCONFIG_MCUDISACKINTST), SR_DISABLE_TIMEOUT,
> +			     timeout);
>  
>  	if (timeout >= SR_DISABLE_TIMEOUT)
>  		dev_warn(&sr->pdev->dev, "%s: Smartreflex disable timedout\n",
> @@ -334,9 +334,9 @@ static void sr_v2_disable(struct omap_sr *sr)
>  	 * Wait for SR to be disabled.
>  	 * wait until IRQSTATUS.MCUDISACKINTST = 1. Typical latency is 1us.
>  	 */
> -	omap_test_timeout((sr_read_reg(sr, IRQSTATUS) &
> -			IRQSTATUS_MCUDISABLEACKINT), SR_DISABLE_TIMEOUT,
> -			timeout);
> +	sr_test_cond_timeout((sr_read_reg(sr, IRQSTATUS) &
> +			     IRQSTATUS_MCUDISABLEACKINT), SR_DISABLE_TIMEOUT,
> +			     timeout);
>  
>  	if (timeout >= SR_DISABLE_TIMEOUT)
>  		dev_warn(&sr->pdev->dev, "%s: Smartreflex disable timedout\n",
> diff --git a/include/linux/power/smartreflex.h b/include/linux/power/smartreflex.h
> index 884eaee..78b795e 100644
> --- a/include/linux/power/smartreflex.h
> +++ b/include/linux/power/smartreflex.h
> @@ -22,7 +22,7 @@
>  
>  #include <linux/types.h>
>  #include <linux/platform_device.h>
> -
> +#include <linux/delay.h>
>  #include <plat/voltage.h>
>  
>  /*
> @@ -168,6 +168,27 @@ struct omap_sr {
>  };
>  
>  /**
> + * test_cond_timeout - busy-loop, testing a condition
> + * @cond: condition to test until it evaluates to true
> + * @timeout: maximum number of microseconds in the timeout
> + * @index: loop index (integer)
> + *
> + * Loop waiting for @cond to become true or until at least @timeout
> + * microseconds have passed.  To use, define some integer @index in the
> + * calling code.  After running, if @index == @timeout, then the loop has
> + * timed out.
> + *
> + * Copied from omap_test_timeout */
> +#define sr_test_cond_timeout(cond, timeout, index)		\
> +({								\
> +	for (index = 0; index < timeout; index++) {		\
> +		if (cond)					\
> +			break;					\
> +		udelay(1);					\
> +	}							\
> +})

I think we can use time_after()/time_before() APIs for timeout and cpu_relax() for
tight loops instead of udelay().

Regards
AnilKumar

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

* Re: [PATCH V3 04/10] ARM: OMAP3: hwmod: rename the smartreflex entries
  2012-05-04  8:30   ` AnilKumar, Chimata
@ 2012-05-04 10:11     ` J, KEERTHY
  2012-05-07 23:39       ` Kevin Hilman
  0 siblings, 1 reply; 49+ messages in thread
From: J, KEERTHY @ 2012-05-04 10:11 UTC (permalink / raw)
  To: AnilKumar, Chimata
  Cc: linux-omap, linux-arm-kernel, Hilman, Kevin, rjw, linux-kernel,
	linux-pm, Pihet-XID, Jean

Hi AnilKumar,

Thanks for reviewing.

On Fri, May 4, 2012 at 2:00 PM, AnilKumar, Chimata <anilkumar@ti.com> wrote:
> On Thu, Apr 26, 2012 at 23:10:35, J, KEERTHY wrote:
>> From: Jean Pihet <j-pihet@ti.com>
>>
>> Change the name field value to better reflect the smartreflex
>> integration in the system.
>>
>> Signed-off-by: Jean Pihet <j-pihet@ti.com>
>> Signed-off-by: J Keerthy <j-keerthy@ti.com>
>> ---
>>  arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |    8 ++++----
>>  arch/arm/mach-omap2/smartreflex.c          |    2 +-
>>  2 files changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
>> index 144d118..15907b0 100644
>> --- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
>> +++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
>> @@ -1324,7 +1324,7 @@ static struct omap_hwmod_irq_info omap3_smartreflex_mpu_irqs[] = {
>>  };
>>
>>  static struct omap_hwmod omap34xx_sr1_hwmod = {
>> -     .name           = "sr1",
>> +     .name           = "smartreflex_mpu_iva",
>>       .class          = &omap34xx_smartreflex_hwmod_class,
>>       .main_clk       = "sr1_fck",
>>       .prcm           = {
>> @@ -1342,7 +1342,7 @@ static struct omap_hwmod omap34xx_sr1_hwmod = {
>>  };
>>
>>  static struct omap_hwmod omap36xx_sr1_hwmod = {
>> -     .name           = "sr1",
>> +     .name           = "smartreflex_mpu_iva",
>>       .class          = &omap36xx_smartreflex_hwmod_class,
>>       .main_clk       = "sr1_fck",
>>       .prcm           = {
>> @@ -1369,7 +1369,7 @@ static struct omap_hwmod_irq_info omap3_smartreflex_core_irqs[] = {
>>  };
>>
>>  static struct omap_hwmod omap34xx_sr2_hwmod = {
>> -     .name           = "sr2",
>> +     .name           = "smartreflex_core",
>>       .class          = &omap34xx_smartreflex_hwmod_class,
>>       .main_clk       = "sr2_fck",
>>       .prcm           = {
>> @@ -1387,7 +1387,7 @@ static struct omap_hwmod omap34xx_sr2_hwmod = {
>>  };
>>
>>  static struct omap_hwmod omap36xx_sr2_hwmod = {
>> -     .name           = "sr2",
>> +     .name           = "smartreflex_core",
>>       .class          = &omap36xx_smartreflex_hwmod_class,
>>       .main_clk       = "sr2_fck",
>>       .prcm           = {
>> diff --git a/arch/arm/mach-omap2/smartreflex.c b/arch/arm/mach-omap2/smartreflex.c
>> index 2edd1e2..d859277 100644
>> --- a/arch/arm/mach-omap2/smartreflex.c
>> +++ b/arch/arm/mach-omap2/smartreflex.c
>> @@ -183,7 +183,7 @@ static void sr_set_regfields(struct omap_sr *sr)
>>               sr->err_weight = OMAP3430_SR_ERRWEIGHT;
>>               sr->err_maxlimit = OMAP3430_SR_ERRMAXLIMIT;
>>               sr->accum_data = OMAP3430_SR_ACCUMDATA;
>> -             if (!(strcmp(sr->name, "sr1"))) {
>> +             if (!(strcmp(sr->name, "smartreflex_mpu_iva"))) {
>
> What if voltage rail is different for mpu and iva? I have seen some devices
> supports SmartReflex have different voltage rails for mpu and iva.
>

I get the point. OMAP3 iva and mpu have a common rail. OMAP4 onwards
even we have different rails for mpu and iva. I will enhance the checks here.

> Regards
> AnilKumar



-- 
Regards and Thanks,
Keerthy

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

* Re: [PATCH V3 05/10] ARM: OMAP2+: SmartReflex: introduce a busy loop condition test macro
  2012-05-04  9:12   ` AnilKumar, Chimata
@ 2012-05-07  5:21     ` J, KEERTHY
  2012-05-08 10:17       ` AnilKumar, Chimata
  0 siblings, 1 reply; 49+ messages in thread
From: J, KEERTHY @ 2012-05-07  5:21 UTC (permalink / raw)
  To: AnilKumar, Chimata
  Cc: linux-omap, linux-arm-kernel, Hilman, Kevin, rjw, linux-kernel,
	linux-pm, Pihet-XID, Jean

On Fri, May 4, 2012 at 2:42 PM, AnilKumar, Chimata <anilkumar@ti.com> wrote:
> On Thu, Apr 26, 2012 at 23:10:36, J, KEERTHY wrote:
>> From: Jean Pihet <j-pihet@ti.com>
>>
>> Now that omap_test_timeout is only accessible from mach-omap2/,
>> introduce a similar function for SR.
>>
>> This change makes the SmartReflex implementation ready for the move
>> to drivers/.
>>
>> Signed-off-by: Jean Pihet <j-pihet@ti.com>
>> Signed-off-by: J Keerthy <j-keerthy@ti.com>
>> ---
>>  arch/arm/mach-omap2/smartreflex.c |   12 ++++++------
>>  include/linux/power/smartreflex.h |   23 ++++++++++++++++++++++-
>>  2 files changed, 28 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/smartreflex.c b/arch/arm/mach-omap2/smartreflex.c
>> index d859277..acef08d 100644
>> --- a/arch/arm/mach-omap2/smartreflex.c
>> +++ b/arch/arm/mach-omap2/smartreflex.c
>> @@ -289,9 +289,9 @@ static void sr_v1_disable(struct omap_sr *sr)
>>        * Wait for SR to be disabled.
>>        * wait until ERRCONFIG.MCUDISACKINTST = 1. Typical latency is 1us.
>>        */
>> -     omap_test_timeout((sr_read_reg(sr, ERRCONFIG_V1) &
>> -                     ERRCONFIG_MCUDISACKINTST), SR_DISABLE_TIMEOUT,
>> -                     timeout);
>> +     sr_test_cond_timeout((sr_read_reg(sr, ERRCONFIG_V1) &
>> +                          ERRCONFIG_MCUDISACKINTST), SR_DISABLE_TIMEOUT,
>> +                          timeout);
>>
>>       if (timeout >= SR_DISABLE_TIMEOUT)
>>               dev_warn(&sr->pdev->dev, "%s: Smartreflex disable timedout\n",
>> @@ -334,9 +334,9 @@ static void sr_v2_disable(struct omap_sr *sr)
>>        * Wait for SR to be disabled.
>>        * wait until IRQSTATUS.MCUDISACKINTST = 1. Typical latency is 1us.
>>        */
>> -     omap_test_timeout((sr_read_reg(sr, IRQSTATUS) &
>> -                     IRQSTATUS_MCUDISABLEACKINT), SR_DISABLE_TIMEOUT,
>> -                     timeout);
>> +     sr_test_cond_timeout((sr_read_reg(sr, IRQSTATUS) &
>> +                          IRQSTATUS_MCUDISABLEACKINT), SR_DISABLE_TIMEOUT,
>> +                          timeout);
>>
>>       if (timeout >= SR_DISABLE_TIMEOUT)
>>               dev_warn(&sr->pdev->dev, "%s: Smartreflex disable timedout\n",
>> diff --git a/include/linux/power/smartreflex.h b/include/linux/power/smartreflex.h
>> index 884eaee..78b795e 100644
>> --- a/include/linux/power/smartreflex.h
>> +++ b/include/linux/power/smartreflex.h
>> @@ -22,7 +22,7 @@
>>
>>  #include <linux/types.h>
>>  #include <linux/platform_device.h>
>> -
>> +#include <linux/delay.h>
>>  #include <plat/voltage.h>
>>
>>  /*
>> @@ -168,6 +168,27 @@ struct omap_sr {
>>  };
>>
>>  /**
>> + * test_cond_timeout - busy-loop, testing a condition
>> + * @cond: condition to test until it evaluates to true
>> + * @timeout: maximum number of microseconds in the timeout
>> + * @index: loop index (integer)
>> + *
>> + * Loop waiting for @cond to become true or until at least @timeout
>> + * microseconds have passed.  To use, define some integer @index in the
>> + * calling code.  After running, if @index == @timeout, then the loop has
>> + * timed out.
>> + *
>> + * Copied from omap_test_timeout */
>> +#define sr_test_cond_timeout(cond, timeout, index)           \
>> +({                                                           \
>> +     for (index = 0; index < timeout; index++) {             \
>> +             if (cond)                                       \
>> +                     break;                                  \
>> +             udelay(1);                                      \
>> +     }                                                       \
>> +})
>
> I think we can use time_after()/time_before() APIs for timeout and cpu_relax() for
> tight loops instead of udelay().

cpu_relax() changes the priority everytime to low and will yield to
another thread.
Considering that we are checking the condition every microsecond does it make
some saving using cpu_relax().

>
> Regards
> AnilKumar



-- 
Regards and Thanks,
Keerthy

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

* Re: [PATCH V3 04/10] ARM: OMAP3: hwmod: rename the smartreflex entries
  2012-05-04 10:11     ` J, KEERTHY
@ 2012-05-07 23:39       ` Kevin Hilman
  2012-05-07 23:55         ` Kevin Hilman
  0 siblings, 1 reply; 49+ messages in thread
From: Kevin Hilman @ 2012-05-07 23:39 UTC (permalink / raw)
  To: J, KEERTHY
  Cc: AnilKumar, Chimata, linux-omap, linux-arm-kernel, rjw,
	linux-kernel, linux-pm, Pihet-XID, Jean

"J, KEERTHY" <j-keerthy@ti.com> writes:

> Hi AnilKumar,
>
> Thanks for reviewing.
>
> On Fri, May 4, 2012 at 2:00 PM, AnilKumar, Chimata <anilkumar@ti.com> wrote:
>> On Thu, Apr 26, 2012 at 23:10:35, J, KEERTHY wrote:
>>> From: Jean Pihet <j-pihet@ti.com>
>>>
>>> Change the name field value to better reflect the smartreflex
>>> integration in the system.
>>>
>>> Signed-off-by: Jean Pihet <j-pihet@ti.com>
>>> Signed-off-by: J Keerthy <j-keerthy@ti.com>
>>> ---
>>>  arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |    8 ++++----
>>>  arch/arm/mach-omap2/smartreflex.c          |    2 +-
>>>  2 files changed, 5 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
>>> index 144d118..15907b0 100644
>>> --- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
>>> +++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
>>> @@ -1324,7 +1324,7 @@ static struct omap_hwmod_irq_info omap3_smartreflex_mpu_irqs[] = {
>>>  };
>>>
>>>  static struct omap_hwmod omap34xx_sr1_hwmod = {
>>> -     .name           = "sr1",
>>> +     .name           = "smartreflex_mpu_iva",
>>>       .class          = &omap34xx_smartreflex_hwmod_class,
>>>       .main_clk       = "sr1_fck",
>>>       .prcm           = {
>>> @@ -1342,7 +1342,7 @@ static struct omap_hwmod omap34xx_sr1_hwmod = {
>>>  };
>>>
>>>  static struct omap_hwmod omap36xx_sr1_hwmod = {
>>> -     .name           = "sr1",
>>> +     .name           = "smartreflex_mpu_iva",
>>>       .class          = &omap36xx_smartreflex_hwmod_class,
>>>       .main_clk       = "sr1_fck",
>>>       .prcm           = {
>>> @@ -1369,7 +1369,7 @@ static struct omap_hwmod_irq_info omap3_smartreflex_core_irqs[] = {
>>>  };
>>>
>>>  static struct omap_hwmod omap34xx_sr2_hwmod = {
>>> -     .name           = "sr2",
>>> +     .name           = "smartreflex_core",
>>>       .class          = &omap34xx_smartreflex_hwmod_class,
>>>       .main_clk       = "sr2_fck",
>>>       .prcm           = {
>>> @@ -1387,7 +1387,7 @@ static struct omap_hwmod omap34xx_sr2_hwmod = {
>>>  };
>>>
>>>  static struct omap_hwmod omap36xx_sr2_hwmod = {
>>> -     .name           = "sr2",
>>> +     .name           = "smartreflex_core",
>>>       .class          = &omap36xx_smartreflex_hwmod_class,
>>>       .main_clk       = "sr2_fck",
>>>       .prcm           = {
>>> diff --git a/arch/arm/mach-omap2/smartreflex.c b/arch/arm/mach-omap2/smartreflex.c
>>> index 2edd1e2..d859277 100644
>>> --- a/arch/arm/mach-omap2/smartreflex.c
>>> +++ b/arch/arm/mach-omap2/smartreflex.c
>>> @@ -183,7 +183,7 @@ static void sr_set_regfields(struct omap_sr *sr)
>>>               sr->err_weight = OMAP3430_SR_ERRWEIGHT;
>>>               sr->err_maxlimit = OMAP3430_SR_ERRMAXLIMIT;
>>>               sr->accum_data = OMAP3430_SR_ACCUMDATA;
>>> -             if (!(strcmp(sr->name, "sr1"))) {
>>> +             if (!(strcmp(sr->name, "smartreflex_mpu_iva"))) {
>>
>> What if voltage rail is different for mpu and iva? I have seen some devices
>> supports SmartReflex have different voltage rails for mpu and iva.
>>
>
> I get the point. OMAP3 iva and mpu have a common rail. OMAP4 onwards
> even we have different rails for mpu and iva. I will enhance the checks here.

Rather than enhancing the checks, this SoC specific data should probably
just be made part of the SoC specific hwmod dev_attr.

Kevin


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

* Re: [PATCH V3 00/10] PM: Create the AVS(Adaptive Voltage Scaling)
  2012-05-04  8:21         ` AnilKumar, Chimata
@ 2012-05-07 23:48           ` Kevin Hilman
  2012-05-08  3:48             ` J, KEERTHY
  2012-05-08 10:17             ` AnilKumar, Chimata
  0 siblings, 2 replies; 49+ messages in thread
From: Kevin Hilman @ 2012-05-07 23:48 UTC (permalink / raw)
  To: AnilKumar, Chimata
  Cc: Mark Brown, J, KEERTHY, linux-omap, linux-arm-kernel, rjw,
	linux-kernel, linux-pm, Pihet-XID, Jean

"AnilKumar, Chimata" <anilkumar@ti.com> writes:

> On Sat, Apr 28, 2012 at 02:31:17, Hilman, Kevin wrote:
>> Hi Mark,
>> 
>> Mark Brown <broonie@opensource.wolfsonmicro.com> writes:
>> 
>> > On Fri, Apr 27, 2012 at 11:09:10AM +0530, J, KEERTHY wrote:
>> >
>> >> Devfreq and cpufreq are related to dynamic frequency/voltage switching between
>> >> pre defined Operating Performance Points or the OPPs. Every OPP being
>> >> a voltage/frequency pair. Smartreflex is a different
>> >> power management technique.
>> >
>> > But presumably these things should integrate somehow - for example,
>> > should devfreq and cpufreq be providing inputs into what AVS is doing,
>> > and if so how?
>> 
>> The way it is currently designed, cpufreq/devfreq/regulator layers don't
>> need to know about AVS.
>> 
>> The higher-level layers only know about the "nominal" voltage.  AVS
>> hardware does automatic, adaptive, micro-adjustments around that nominal
>> voltage, and these micro-adjustments are managed by the AVS hardware
>> sending commands to the PMIC.  (specifically, on OMAP, the AVS sensors
>> provide inputs to the voltage processor (VP) which provide inputs to the
>> voltage controller (VC) which sends commands to the PMIC[1].)
>> 
>> The driver proposed here is primarily for initializing the various
>> parameters/sensitivity/etc. of the AVS hardware, but the actual voltage
>> adjustments are done in hardware by VC/VP.
>> 
>> The only thing the higher-level layers might potentially need to do to
>> enable/disable AVS around transitions (e.g. when changing OPP, AVS is
>> disabled before changing OPP and only re-enabled when the new nominal
>> voltage has been acheived.)
>> 
>> On OMAP, we handle this inside the OMAP-specific voltage layer which is
>> called by the regulator framework, so even the regulators do not need
>> any knowledge of AVS.
>
> Kevin,
>
> I want to point out some cases of SR implementation where this may not
> be true.
>
> Devices like DM8168, DM8148 and AM335X use Class 2B implementation of SR.
>
> Under this, SR module issues an interrupt to ARM when there is a need to
> change the voltage based on temperature changes, ageing etc.
>
> Once the interrupt arrives, kernel needs to adjust voltage using regulator API.
> The voltage change is a micro adjustment as in other SR classes.

That can easily be handled writing a plugin specific to class 2B.  This
driver was designed so plugins for other classes can be supported.  

Sure, we might need some enhancements for other classes (we already know
that we will for class 1 support.)  However, the purpose of this series
is to do the cleanups necessary for the driver to move to drivers/*.

Support for additional classes can be added after the driver is moved
if/when folks are motivated to post that support upstream.

> The SR class 2B implementation on these devices does not exist in mainline.
> I can point to some public repositories if you are interested in taking a look at
> the current code.

No thanks.  We can discuss it when you post support for it to mainline.

Kevin

> Implementation of this SR method is must on at least the DM8168 device and
> I know some customers who are using it on their production systems.
>
> Regards
> AnilKumar
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

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

* Re: [PATCH V3 00/10] PM: Create the AVS(Adaptive Voltage Scaling)
  2012-04-26 17:40 [PATCH V3 00/10] PM: Create the AVS(Adaptive Voltage Scaling) Keerthy
                   ` (10 preceding siblings ...)
  2012-04-26 19:11 ` [PATCH V3 00/10] PM: Create the AVS(Adaptive Voltage Scaling) Mark Brown
@ 2012-05-07 23:51 ` Kevin Hilman
  2012-05-15  5:46   ` J, KEERTHY
  11 siblings, 1 reply; 49+ messages in thread
From: Kevin Hilman @ 2012-05-07 23:51 UTC (permalink / raw)
  To: Keerthy, Rafael J. Wysocki
  Cc: linux-omap, linux-arm-kernel, rjw, linux-kernel, linux-pm, j-pihet

Rafael,

Keerthy <j-keerthy@ti.com> writes:

> From: J Keerthy <j-keerthy@ti.com>
>
> AVS(Adaptive Voltage Scaling) is a power management technique which
> controls the operating voltage of a device in order to optimize (i.e. reduce)
> its power consumption. The voltage is adapted depending on static factors
> (chip manufacturing process) and dynamic factors (temperature
> depending performance).
> The TI AVS solution is named Smartreflex. 
>
> To that end, create the AVS driver in drivers/power/avs and
> move the OMAP SmartReflex code to the new directory. The
> class driver is still retained in the mach-omap2 directory.

How should we handle this for upstream?

It does a bunch of cleanup under arch/arm then does the move to
drivers/power the end.  To avoid conflicts with other OMAP core changes,
I would suggest we take this through the OMAP tree.

With your ack, I'd be glad to take it.

Thanks,

Kevin


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

* Re: [PATCH V3 04/10] ARM: OMAP3: hwmod: rename the smartreflex entries
  2012-05-07 23:39       ` Kevin Hilman
@ 2012-05-07 23:55         ` Kevin Hilman
  2012-05-08  3:44           ` J, KEERTHY
  0 siblings, 1 reply; 49+ messages in thread
From: Kevin Hilman @ 2012-05-07 23:55 UTC (permalink / raw)
  To: J, KEERTHY
  Cc: AnilKumar, Chimata, linux-omap, linux-arm-kernel, rjw,
	linux-kernel, linux-pm, Pihet-XID, Jean

Kevin Hilman <khilman@ti.com> writes:

> "J, KEERTHY" <j-keerthy@ti.com> writes:

[...]

>>>> diff --git a/arch/arm/mach-omap2/smartreflex.c b/arch/arm/mach-omap2/smartreflex.c
>>>> index 2edd1e2..d859277 100644
>>>> --- a/arch/arm/mach-omap2/smartreflex.c
>>>> +++ b/arch/arm/mach-omap2/smartreflex.c
>>>> @@ -183,7 +183,7 @@ static void sr_set_regfields(struct omap_sr *sr)
>>>>               sr->err_weight = OMAP3430_SR_ERRWEIGHT;
>>>>               sr->err_maxlimit = OMAP3430_SR_ERRMAXLIMIT;
>>>>               sr->accum_data = OMAP3430_SR_ACCUMDATA;
>>>> -             if (!(strcmp(sr->name, "sr1"))) {
>>>> +             if (!(strcmp(sr->name, "smartreflex_mpu_iva"))) {
>>>
>>> What if voltage rail is different for mpu and iva? I have seen some devices
>>> supports SmartReflex have different voltage rails for mpu and iva.
>>>
>>
>> I get the point. OMAP3 iva and mpu have a common rail. OMAP4 onwards
>> even we have different rails for mpu and iva. I will enhance the checks here.
>
> Rather than enhancing the checks, this SoC specific data should probably
> just be made part of the SoC specific hwmod dev_attr.

That being said, this is an additional feature we can add after this
driver is moved.

I would like this series to concentrate on the cleanups necessary to
move to drivers/*, then additional features to support other SoCs can be
added on top.

Keerthy, please prepare a patch to generalize this to other SoCs by
using dev_attr for this SoC specific data.   We can add it after this
series is merged upstream.

Thanks,

Kevin

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

* Re: [PATCH V3 04/10] ARM: OMAP3: hwmod: rename the smartreflex entries
  2012-05-07 23:55         ` Kevin Hilman
@ 2012-05-08  3:44           ` J, KEERTHY
  0 siblings, 0 replies; 49+ messages in thread
From: J, KEERTHY @ 2012-05-08  3:44 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: AnilKumar, Chimata, linux-omap, linux-arm-kernel, rjw,
	linux-kernel, linux-pm, Pihet-XID, Jean

On Tue, May 8, 2012 at 5:25 AM, Kevin Hilman <khilman@ti.com> wrote:
> Kevin Hilman <khilman@ti.com> writes:
>
>> "J, KEERTHY" <j-keerthy@ti.com> writes:
>
> [...]
>
>>>>> diff --git a/arch/arm/mach-omap2/smartreflex.c b/arch/arm/mach-omap2/smartreflex.c
>>>>> index 2edd1e2..d859277 100644
>>>>> --- a/arch/arm/mach-omap2/smartreflex.c
>>>>> +++ b/arch/arm/mach-omap2/smartreflex.c
>>>>> @@ -183,7 +183,7 @@ static void sr_set_regfields(struct omap_sr *sr)
>>>>>               sr->err_weight = OMAP3430_SR_ERRWEIGHT;
>>>>>               sr->err_maxlimit = OMAP3430_SR_ERRMAXLIMIT;
>>>>>               sr->accum_data = OMAP3430_SR_ACCUMDATA;
>>>>> -             if (!(strcmp(sr->name, "sr1"))) {
>>>>> +             if (!(strcmp(sr->name, "smartreflex_mpu_iva"))) {
>>>>
>>>> What if voltage rail is different for mpu and iva? I have seen some devices
>>>> supports SmartReflex have different voltage rails for mpu and iva.
>>>>
>>>
>>> I get the point. OMAP3 iva and mpu have a common rail. OMAP4 onwards
>>> even we have different rails for mpu and iva. I will enhance the checks here.
>>
>> Rather than enhancing the checks, this SoC specific data should probably
>> just be made part of the SoC specific hwmod dev_attr.
>
> That being said, this is an additional feature we can add after this
> driver is moved.
>
> I would like this series to concentrate on the cleanups necessary to
> move to drivers/*, then additional features to support other SoCs can be
> added on top.
>
> Keerthy, please prepare a patch to generalize this to other SoCs by
> using dev_attr for this SoC specific data.   We can add it after this
> series is merged upstream.
Kevin,

Ok. I will do that.

>
> Thanks,
>
> Kevin



-- 
Regards and Thanks,
Keerthy

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

* Re: [PATCH V3 00/10] PM: Create the AVS(Adaptive Voltage Scaling)
  2012-05-07 23:48           ` Kevin Hilman
@ 2012-05-08  3:48             ` J, KEERTHY
  2012-05-08 10:17             ` AnilKumar, Chimata
  1 sibling, 0 replies; 49+ messages in thread
From: J, KEERTHY @ 2012-05-08  3:48 UTC (permalink / raw)
  To: Kevin Hilman, AnilKumar, Chimata
  Cc: Mark Brown, linux-omap, linux-arm-kernel, rjw, linux-kernel,
	linux-pm, Pihet-XID, Jean

On Tue, May 8, 2012 at 5:18 AM, Kevin Hilman <khilman@ti.com> wrote:
> "AnilKumar, Chimata" <anilkumar@ti.com> writes:
>
>> On Sat, Apr 28, 2012 at 02:31:17, Hilman, Kevin wrote:
>>> Hi Mark,
>>>
>>> Mark Brown <broonie@opensource.wolfsonmicro.com> writes:
>>>
>>> > On Fri, Apr 27, 2012 at 11:09:10AM +0530, J, KEERTHY wrote:
>>> >
>>> >> Devfreq and cpufreq are related to dynamic frequency/voltage switching between
>>> >> pre defined Operating Performance Points or the OPPs. Every OPP being
>>> >> a voltage/frequency pair. Smartreflex is a different
>>> >> power management technique.
>>> >
>>> > But presumably these things should integrate somehow - for example,
>>> > should devfreq and cpufreq be providing inputs into what AVS is doing,
>>> > and if so how?
>>>
>>> The way it is currently designed, cpufreq/devfreq/regulator layers don't
>>> need to know about AVS.
>>>
>>> The higher-level layers only know about the "nominal" voltage.  AVS
>>> hardware does automatic, adaptive, micro-adjustments around that nominal
>>> voltage, and these micro-adjustments are managed by the AVS hardware
>>> sending commands to the PMIC.  (specifically, on OMAP, the AVS sensors
>>> provide inputs to the voltage processor (VP) which provide inputs to the
>>> voltage controller (VC) which sends commands to the PMIC[1].)
>>>
>>> The driver proposed here is primarily for initializing the various
>>> parameters/sensitivity/etc. of the AVS hardware, but the actual voltage
>>> adjustments are done in hardware by VC/VP.
>>>
>>> The only thing the higher-level layers might potentially need to do to
>>> enable/disable AVS around transitions (e.g. when changing OPP, AVS is
>>> disabled before changing OPP and only re-enabled when the new nominal
>>> voltage has been acheived.)
>>>
>>> On OMAP, we handle this inside the OMAP-specific voltage layer which is
>>> called by the regulator framework, so even the regulators do not need
>>> any knowledge of AVS.
>>
>> Kevin,
>>
>> I want to point out some cases of SR implementation where this may not
>> be true.
>>
>> Devices like DM8168, DM8148 and AM335X use Class 2B implementation of SR.
>>
>> Under this, SR module issues an interrupt to ARM when there is a need to
>> change the voltage based on temperature changes, ageing etc.
>>
>> Once the interrupt arrives, kernel needs to adjust voltage using regulator API.
>> The voltage change is a micro adjustment as in other SR classes.
>
> That can easily be handled writing a plugin specific to class 2B.  This
> driver was designed so plugins for other classes can be supported.
>
> Sure, we might need some enhancements for other classes (we already know
> that we will for class 1 support.)  However, the purpose of this series
> is to do the cleanups necessary for the driver to move to drivers/*.

AnilKumar,

The intent of the series as explained by Kevin if to do the necessary clean up
for the driver to move from mach-omap to drivers/*. We will for sure need
more enhancements for other classes support.

>
> Support for additional classes can be added after the driver is moved
> if/when folks are motivated to post that support upstream.
>
>> The SR class 2B implementation on these devices does not exist in mainline.
>> I can point to some public repositories if you are interested in taking a look at
>> the current code.
>
> No thanks.  We can discuss it when you post support for it to mainline.
>
> Kevin
>
>> Implementation of this SR method is must on at least the DM8168 device and
>> I know some customers who are using it on their production systems.
>>
>> Regards
>> AnilKumar
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/



-- 
Regards and Thanks,
Keerthy

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

* RE: [PATCH V3 05/10] ARM: OMAP2+: SmartReflex: introduce a busy loop condition test macro
  2012-05-07  5:21     ` J, KEERTHY
@ 2012-05-08 10:17       ` AnilKumar, Chimata
  2012-05-10  6:19         ` J, KEERTHY
  0 siblings, 1 reply; 49+ messages in thread
From: AnilKumar, Chimata @ 2012-05-08 10:17 UTC (permalink / raw)
  To: J, KEERTHY
  Cc: linux-omap, linux-arm-kernel, Hilman, Kevin, rjw, linux-kernel,
	linux-pm, Pihet-XID, Jean

On Mon, May 07, 2012 at 10:51:53, J, KEERTHY wrote:
> On Fri, May 4, 2012 at 2:42 PM, AnilKumar, Chimata <anilkumar@ti.com> wrote:
> > On Thu, Apr 26, 2012 at 23:10:36, J, KEERTHY wrote:
> >> From: Jean Pihet <j-pihet@ti.com>
> >>
> >> Now that omap_test_timeout is only accessible from mach-omap2/,
> >> introduce a similar function for SR.
> >>
> >> This change makes the SmartReflex implementation ready for the move
> >> to drivers/.
> >>
> >> Signed-off-by: Jean Pihet <j-pihet@ti.com>
> >> Signed-off-by: J Keerthy <j-keerthy@ti.com>
> >> ---
> >>  arch/arm/mach-omap2/smartreflex.c |   12 ++++++------
> >>  include/linux/power/smartreflex.h |   23 ++++++++++++++++++++++-
> >>  2 files changed, 28 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/arch/arm/mach-omap2/smartreflex.c b/arch/arm/mach-omap2/smartreflex.c
> >> index d859277..acef08d 100644
> >> --- a/arch/arm/mach-omap2/smartreflex.c
> >> +++ b/arch/arm/mach-omap2/smartreflex.c
> >> @@ -289,9 +289,9 @@ static void sr_v1_disable(struct omap_sr *sr)
> >>        * Wait for SR to be disabled.
> >>        * wait until ERRCONFIG.MCUDISACKINTST = 1. Typical latency is 1us.
> >>        */
> >> -     omap_test_timeout((sr_read_reg(sr, ERRCONFIG_V1) &
> >> -                     ERRCONFIG_MCUDISACKINTST), SR_DISABLE_TIMEOUT,
> >> -                     timeout);
> >> +     sr_test_cond_timeout((sr_read_reg(sr, ERRCONFIG_V1) &
> >> +                          ERRCONFIG_MCUDISACKINTST), SR_DISABLE_TIMEOUT,
> >> +                          timeout);
> >>
> >>       if (timeout >= SR_DISABLE_TIMEOUT)
> >>               dev_warn(&sr->pdev->dev, "%s: Smartreflex disable timedout\n",
> >> @@ -334,9 +334,9 @@ static void sr_v2_disable(struct omap_sr *sr)
> >>        * Wait for SR to be disabled.
> >>        * wait until IRQSTATUS.MCUDISACKINTST = 1. Typical latency is 1us.
> >>        */
> >> -     omap_test_timeout((sr_read_reg(sr, IRQSTATUS) &
> >> -                     IRQSTATUS_MCUDISABLEACKINT), SR_DISABLE_TIMEOUT,
> >> -                     timeout);
> >> +     sr_test_cond_timeout((sr_read_reg(sr, IRQSTATUS) &
> >> +                          IRQSTATUS_MCUDISABLEACKINT), SR_DISABLE_TIMEOUT,
> >> +                          timeout);
> >>
> >>       if (timeout >= SR_DISABLE_TIMEOUT)
> >>               dev_warn(&sr->pdev->dev, "%s: Smartreflex disable timedout\n",
> >> diff --git a/include/linux/power/smartreflex.h b/include/linux/power/smartreflex.h
> >> index 884eaee..78b795e 100644
> >> --- a/include/linux/power/smartreflex.h
> >> +++ b/include/linux/power/smartreflex.h
> >> @@ -22,7 +22,7 @@
> >>
> >>  #include <linux/types.h>
> >>  #include <linux/platform_device.h>
> >> -
> >> +#include <linux/delay.h>
> >>  #include <plat/voltage.h>
> >>
> >>  /*
> >> @@ -168,6 +168,27 @@ struct omap_sr {
> >>  };
> >>
> >>  /**
> >> + * test_cond_timeout - busy-loop, testing a condition
> >> + * @cond: condition to test until it evaluates to true
> >> + * @timeout: maximum number of microseconds in the timeout
> >> + * @index: loop index (integer)
> >> + *
> >> + * Loop waiting for @cond to become true or until at least @timeout
> >> + * microseconds have passed.  To use, define some integer @index in the
> >> + * calling code.  After running, if @index == @timeout, then the loop has
> >> + * timed out.
> >> + *
> >> + * Copied from omap_test_timeout */
> >> +#define sr_test_cond_timeout(cond, timeout, index)           \
> >> +({                                                           \
> >> +     for (index = 0; index < timeout; index++) {             \
> >> +             if (cond)                                       \
> >> +                     break;                                  \
> >> +             udelay(1);                                      \
> >> +     }                                                       \
> >> +})
> >
> > I think we can use time_after()/time_before() APIs for timeout and cpu_relax() for
> > tight loops instead of udelay().
> 
> cpu_relax() changes the priority everytime to low and will yield to
> another thread.
> Considering that we are checking the condition every microsecond does it make
> some saving using cpu_relax().
> 

cpu_relax() does not involve any priority changes or scheduling AFAICS.
Have a look at this thread:

http://www.spinics.net/lists/netdev/msg151699.html

Regards
AnilKumar


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

* RE: [PATCH V3 00/10] PM: Create the AVS(Adaptive Voltage Scaling)
  2012-05-07 23:48           ` Kevin Hilman
  2012-05-08  3:48             ` J, KEERTHY
@ 2012-05-08 10:17             ` AnilKumar, Chimata
  2012-05-08 20:38               ` Woodruff, Richard
  1 sibling, 1 reply; 49+ messages in thread
From: AnilKumar, Chimata @ 2012-05-08 10:17 UTC (permalink / raw)
  To: Hilman, Kevin
  Cc: Mark Brown, J, KEERTHY, linux-omap, linux-arm-kernel, rjw,
	linux-kernel, linux-pm, Pihet-XID, Jean

On Tue, May 08, 2012 at 05:18:41, Hilman, Kevin wrote:
> "AnilKumar, Chimata" <anilkumar@ti.com> writes:
> 
> > On Sat, Apr 28, 2012 at 02:31:17, Hilman, Kevin wrote:
> >> Hi Mark,
> >> 
> >> Mark Brown <broonie@opensource.wolfsonmicro.com> writes:
> >> 
> >> > On Fri, Apr 27, 2012 at 11:09:10AM +0530, J, KEERTHY wrote:
> >> >
> >> >> Devfreq and cpufreq are related to dynamic frequency/voltage switching between
> >> >> pre defined Operating Performance Points or the OPPs. Every OPP being
> >> >> a voltage/frequency pair. Smartreflex is a different
> >> >> power management technique.
> >> >
> >> > But presumably these things should integrate somehow - for example,
> >> > should devfreq and cpufreq be providing inputs into what AVS is doing,
> >> > and if so how?
> >> 
> >> The way it is currently designed, cpufreq/devfreq/regulator layers don't
> >> need to know about AVS.
> >> 
> >> The higher-level layers only know about the "nominal" voltage.  AVS
> >> hardware does automatic, adaptive, micro-adjustments around that nominal
> >> voltage, and these micro-adjustments are managed by the AVS hardware
> >> sending commands to the PMIC.  (specifically, on OMAP, the AVS sensors
> >> provide inputs to the voltage processor (VP) which provide inputs to the
> >> voltage controller (VC) which sends commands to the PMIC[1].)
> >> 
> >> The driver proposed here is primarily for initializing the various
> >> parameters/sensitivity/etc. of the AVS hardware, but the actual voltage
> >> adjustments are done in hardware by VC/VP.
> >> 
> >> The only thing the higher-level layers might potentially need to do to
> >> enable/disable AVS around transitions (e.g. when changing OPP, AVS is
> >> disabled before changing OPP and only re-enabled when the new nominal
> >> voltage has been acheived.)
> >> 
> >> On OMAP, we handle this inside the OMAP-specific voltage layer which is
> >> called by the regulator framework, so even the regulators do not need
> >> any knowledge of AVS.
> >
> > Kevin,
> >
> > I want to point out some cases of SR implementation where this may not
> > be true.
> >
> > Devices like DM8168, DM8148 and AM335X use Class 2B implementation of SR.
> >
> > Under this, SR module issues an interrupt to ARM when there is a need to
> > change the voltage based on temperature changes, ageing etc.
> >
> > Once the interrupt arrives, kernel needs to adjust voltage using regulator API.
> > The voltage change is a micro adjustment as in other SR classes.
> 
> That can easily be handled writing a plugin specific to class 2B.  This
> driver was designed so plugins for other classes can be supported.  
> 
> Sure, we might need some enhancements for other classes (we already know
> that we will for class 1 support.)  However, the purpose of this series
> is to do the cleanups necessary for the driver to move to drivers/*.
> 

It's perfectly fine with me. My intention was just to highlight that
class 2B SR will have to interact with regulator layer for voltage
changes, so I guess it is little different from other SR classes.

Thanks,
AnilKumar


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

* RE: [PATCH V3 00/10] PM: Create the AVS(Adaptive Voltage Scaling)
  2012-05-08 10:17             ` AnilKumar, Chimata
@ 2012-05-08 20:38               ` Woodruff, Richard
  2012-05-08 22:16                 ` [linux-pm] " Kevin Hilman
  0 siblings, 1 reply; 49+ messages in thread
From: Woodruff, Richard @ 2012-05-08 20:38 UTC (permalink / raw)
  To: AnilKumar, Chimata, Hilman, Kevin
  Cc: Mark Brown, J, KEERTHY, linux-omap, linux-arm-kernel, rjw,
	linux-kernel, linux-pm, Pihet-XID, Jean


> > >> The only thing the higher-level layers might potentially need to do
> > >> is to enable/disable AVS around transitions (e.g. when changing OPP, > > >> AVS is disabled before changing OPP and only re-enabled when the new > >> >> nominal voltage has been acheived.)

Getting clean baseline in place is huge step but actual production interfaces will need to comprehend some OPP to AVS dependencies beyond on/off.

AVS as used in many OMAP designs (mostly > OMAP4430) do have some per OPP dependent details:

- Multiple PMICs are in use. In some current designs the AVS step size is different per voltage range. At OPP change time you have to reconfigure several AVS parameters before enable.

- ABB-ldo sequencing and parameters in tightly coupled with OPP and AVS enables.

- Good power savings can be had in future 1.5/3.5 by adjusting nominal to calibrated-plus-margin voltage.

Regards,
Richard W.


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

* Re: [linux-pm] [PATCH V3 00/10] PM: Create the AVS(Adaptive Voltage Scaling)
  2012-05-08 20:38               ` Woodruff, Richard
@ 2012-05-08 22:16                 ` Kevin Hilman
  2012-05-09  0:39                   ` Woodruff, Richard
  0 siblings, 1 reply; 49+ messages in thread
From: Kevin Hilman @ 2012-05-08 22:16 UTC (permalink / raw)
  To: Woodruff, Richard
  Cc: AnilKumar, Chimata, J, KEERTHY, Mark Brown, linux-kernel,
	linux-pm, linux-omap, Pihet-XID, Jean, linux-arm-kernel

"Woodruff, Richard" <r-woodruff2@ti.com> writes:

>> > >> The only thing the higher-level layers might potentially need to
>> > >> do is to enable/disable AVS around transitions (e.g. when
>> > >> changing OPP, > > >> AVS is disabled before changing OPP and
>> > >> only re-enabled when the new > >> >> nominal voltage has been
>> > >> acheived.)
>
> Getting clean baseline in place is huge step but actual production
> interfaces will need to comprehend some OPP to AVS dependencies beyond
> on/off.

A basic OMAP AVS driver has been in mainline for a long time, yet we
have not seen support submitted for all of these features.

When folks are motivated to propose such changes upstream, we will be
happy to discuss them and add support for them to the AVS driver.

Until then, the primary purpose of this series is to do some minimal
cleanup of an *existing* driver so it can be moved into drivers/*.  New
features can be added there as easily as they could've been added when
it was a driver under arch/arm.

Kevin


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

* RE: [linux-pm] [PATCH V3 00/10] PM: Create the AVS(Adaptive Voltage Scaling)
  2012-05-08 22:16                 ` [linux-pm] " Kevin Hilman
@ 2012-05-09  0:39                   ` Woodruff, Richard
  2012-05-09  8:19                     ` Koen Kooi
  2012-05-09 18:29                     ` Kevin Hilman
  0 siblings, 2 replies; 49+ messages in thread
From: Woodruff, Richard @ 2012-05-09  0:39 UTC (permalink / raw)
  To: Hilman, Kevin
  Cc: AnilKumar, Chimata, J, KEERTHY, Mark Brown, linux-kernel,
	linux-pm, linux-omap, Pihet-XID, Jean, linux-arm-kernel

> From: Hilman, Kevin
> Sent: Tuesday, May 08, 2012 5:17 PM

> A basic OMAP AVS driver has been in mainline for a long time, yet we
> have not seen support submitted for all of these features.

1.5/3.5 is a feature.

ABB is requirement for a production useable driver. Higher speed rated OMAP4 and all OMAP5 added these to be useable. Yes this is effort. Point of mentioning is to raise awareness of need.

Yet to be added feature has different meaning than functional gap.

Regards,
Richard W.


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

* Re: [linux-pm] [PATCH V3 00/10] PM: Create the AVS(Adaptive Voltage Scaling)
  2012-05-09  0:39                   ` Woodruff, Richard
@ 2012-05-09  8:19                     ` Koen Kooi
  2012-05-09 18:29                     ` Kevin Hilman
  1 sibling, 0 replies; 49+ messages in thread
From: Koen Kooi @ 2012-05-09  8:19 UTC (permalink / raw)
  To: Woodruff, Richard
  Cc: Hilman, Kevin, AnilKumar, Chimata, J, KEERTHY, Mark Brown,
	linux-kernel, linux-pm, linux-omap, Pihet-XID, Jean,
	linux-arm-kernel


Op 9 mei 2012, om 02:39 heeft Woodruff, Richard het volgende geschreven:

>> From: Hilman, Kevin
>> Sent: Tuesday, May 08, 2012 5:17 PM
> 
>> A basic OMAP AVS driver has been in mainline for a long time, yet we
>> have not seen support submitted for all of these features.
> 
> 1.5/3.5 is a feature.
> 
> ABB is requirement for a production useable driver. 

ABB/FBB is also needed for 1GHz support on omap3 based SoCs like AM37xx and DM37xx.

regards,

Koen

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

* Re: [linux-pm] [PATCH V3 00/10] PM: Create the AVS(Adaptive Voltage Scaling)
  2012-05-09  0:39                   ` Woodruff, Richard
  2012-05-09  8:19                     ` Koen Kooi
@ 2012-05-09 18:29                     ` Kevin Hilman
  2012-05-23 13:27                       ` Menon, Nishanth
  1 sibling, 1 reply; 49+ messages in thread
From: Kevin Hilman @ 2012-05-09 18:29 UTC (permalink / raw)
  To: Woodruff, Richard
  Cc: J, KEERTHY, Mark Brown, linux-kernel, AnilKumar, Chimata,
	linux-pm, linux-omap, Pihet-XID, Jean, linux-arm-kernel

"Woodruff, Richard" <r-woodruff2@ti.com> writes:

>> From: Hilman, Kevin
>> Sent: Tuesday, May 08, 2012 5:17 PM
>
>> A basic OMAP AVS driver has been in mainline for a long time, yet we
>> have not seen support submitted for all of these features.
>
> 1.5/3.5 is a feature.

And I'm still waiting for it to be submitted upstream.

> ABB is requirement for a production useable driver. Higher speed rated
> OMAP4 and all OMAP5 added these to be useable. 

ditto

> Yes this is effort. Point of mentioning is to raise awareness of need.

I'm well aware of the need.

> Yet to be added feature has different meaning than functional gap.

And both need to be submitted upstream.

Kevin

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

* Re: [PATCH V3 05/10] ARM: OMAP2+: SmartReflex: introduce a busy loop condition test macro
  2012-05-08 10:17       ` AnilKumar, Chimata
@ 2012-05-10  6:19         ` J, KEERTHY
  0 siblings, 0 replies; 49+ messages in thread
From: J, KEERTHY @ 2012-05-10  6:19 UTC (permalink / raw)
  To: AnilKumar, Chimata
  Cc: linux-omap, linux-arm-kernel, Hilman, Kevin, rjw, linux-kernel,
	linux-pm, Pihet-XID, Jean

On Tue, May 8, 2012 at 3:47 PM, AnilKumar, Chimata <anilkumar@ti.com> wrote:
> On Mon, May 07, 2012 at 10:51:53, J, KEERTHY wrote:
>> On Fri, May 4, 2012 at 2:42 PM, AnilKumar, Chimata <anilkumar@ti.com> wrote:
>> > On Thu, Apr 26, 2012 at 23:10:36, J, KEERTHY wrote:
>> >> From: Jean Pihet <j-pihet@ti.com>
>> >>
>> >> Now that omap_test_timeout is only accessible from mach-omap2/,
>> >> introduce a similar function for SR.
>> >>
>> >> This change makes the SmartReflex implementation ready for the move
>> >> to drivers/.
>> >>
>> >> Signed-off-by: Jean Pihet <j-pihet@ti.com>
>> >> Signed-off-by: J Keerthy <j-keerthy@ti.com>
>> >> ---
>> >>  arch/arm/mach-omap2/smartreflex.c |   12 ++++++------
>> >>  include/linux/power/smartreflex.h |   23 ++++++++++++++++++++++-
>> >>  2 files changed, 28 insertions(+), 7 deletions(-)
>> >>
>> >> diff --git a/arch/arm/mach-omap2/smartreflex.c b/arch/arm/mach-omap2/smartreflex.c
>> >> index d859277..acef08d 100644
>> >> --- a/arch/arm/mach-omap2/smartreflex.c
>> >> +++ b/arch/arm/mach-omap2/smartreflex.c
>> >> @@ -289,9 +289,9 @@ static void sr_v1_disable(struct omap_sr *sr)
>> >>        * Wait for SR to be disabled.
>> >>        * wait until ERRCONFIG.MCUDISACKINTST = 1. Typical latency is 1us.
>> >>        */
>> >> -     omap_test_timeout((sr_read_reg(sr, ERRCONFIG_V1) &
>> >> -                     ERRCONFIG_MCUDISACKINTST), SR_DISABLE_TIMEOUT,
>> >> -                     timeout);
>> >> +     sr_test_cond_timeout((sr_read_reg(sr, ERRCONFIG_V1) &
>> >> +                          ERRCONFIG_MCUDISACKINTST), SR_DISABLE_TIMEOUT,
>> >> +                          timeout);
>> >>
>> >>       if (timeout >= SR_DISABLE_TIMEOUT)
>> >>               dev_warn(&sr->pdev->dev, "%s: Smartreflex disable timedout\n",
>> >> @@ -334,9 +334,9 @@ static void sr_v2_disable(struct omap_sr *sr)
>> >>        * Wait for SR to be disabled.
>> >>        * wait until IRQSTATUS.MCUDISACKINTST = 1. Typical latency is 1us.
>> >>        */
>> >> -     omap_test_timeout((sr_read_reg(sr, IRQSTATUS) &
>> >> -                     IRQSTATUS_MCUDISABLEACKINT), SR_DISABLE_TIMEOUT,
>> >> -                     timeout);
>> >> +     sr_test_cond_timeout((sr_read_reg(sr, IRQSTATUS) &
>> >> +                          IRQSTATUS_MCUDISABLEACKINT), SR_DISABLE_TIMEOUT,
>> >> +                          timeout);
>> >>
>> >>       if (timeout >= SR_DISABLE_TIMEOUT)
>> >>               dev_warn(&sr->pdev->dev, "%s: Smartreflex disable timedout\n",
>> >> diff --git a/include/linux/power/smartreflex.h b/include/linux/power/smartreflex.h
>> >> index 884eaee..78b795e 100644
>> >> --- a/include/linux/power/smartreflex.h
>> >> +++ b/include/linux/power/smartreflex.h
>> >> @@ -22,7 +22,7 @@
>> >>
>> >>  #include <linux/types.h>
>> >>  #include <linux/platform_device.h>
>> >> -
>> >> +#include <linux/delay.h>
>> >>  #include <plat/voltage.h>
>> >>
>> >>  /*
>> >> @@ -168,6 +168,27 @@ struct omap_sr {
>> >>  };
>> >>
>> >>  /**
>> >> + * test_cond_timeout - busy-loop, testing a condition
>> >> + * @cond: condition to test until it evaluates to true
>> >> + * @timeout: maximum number of microseconds in the timeout
>> >> + * @index: loop index (integer)
>> >> + *
>> >> + * Loop waiting for @cond to become true or until at least @timeout
>> >> + * microseconds have passed.  To use, define some integer @index in the
>> >> + * calling code.  After running, if @index == @timeout, then the loop has
>> >> + * timed out.
>> >> + *
>> >> + * Copied from omap_test_timeout */
>> >> +#define sr_test_cond_timeout(cond, timeout, index)           \
>> >> +({                                                           \
>> >> +     for (index = 0; index < timeout; index++) {             \
>> >> +             if (cond)                                       \
>> >> +                     break;                                  \
>> >> +             udelay(1);                                      \
>> >> +     }                                                       \
>> >> +})
>> >
>> > I think we can use time_after()/time_before() APIs for timeout and cpu_relax() for
>> > tight loops instead of udelay().
>>
>> cpu_relax() changes the priority everytime to low and will yield to
>> another thread.
>> Considering that we are checking the condition every microsecond does it make
>> some saving using cpu_relax().
>>
>
> cpu_relax() does not involve any priority changes or scheduling AFAICS.
> Have a look at this thread:
>
> http://www.spinics.net/lists/netdev/msg151699.html

Thanks. My bad. I meant yielding to another thread with in a space
of 1uS.

>
> Regards
> AnilKumar
>



-- 
Regards and Thanks,
Keerthy

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

* RE: [PATCH V3 07/10] ARM: OMAP2+: SmartReflex: Use per-OPP data structure
  2012-04-26 17:40 ` [PATCH V3 07/10] ARM: OMAP2+: SmartReflex: Use per-OPP data structure Keerthy
@ 2012-05-10 19:11   ` Guyotte, Greg
  2012-05-11  3:51     ` J, KEERTHY
  0 siblings, 1 reply; 49+ messages in thread
From: Guyotte, Greg @ 2012-05-10 19:11 UTC (permalink / raw)
  To: J, KEERTHY, linux-omap, linux-arm-kernel, Hilman, Kevin, rjw,
	linux-kernel, linux-pm
  Cc: Pihet-XID, Jean, Paul Walmsley, Gopinath, Thara, Menon, Nishanth

Hi Jean,

> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> owner@vger.kernel.org] On Behalf Of J, KEERTHY
> Sent: Thursday, April 26, 2012 12:41 PM
> To: linux-omap@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> Hilman, Kevin; rjw@sisk.pl; linux-kernel@vger.kernel.org; linux-
> pm@lists.linux-foundation.org
> Cc: Pihet-XID, Jean; J, KEERTHY; Paul Walmsley; Gopinath, Thara; Menon,
> Nishanth
> Subject: [PATCH V3 07/10] ARM: OMAP2+: SmartReflex: Use per-OPP data
> structure
> 
> From: Jean Pihet <j-pihet@ti.com>
> 
> The SmartReflex driver incorrectly treats some per-OPP data as data
> common to all OPPs (e.g., ERRMINLIMIT).  Move this data into a per-OPP
> data structure.
> 
> Furthermore, in order to make the SmartReflex implementation ready for
> the move to drivers/, remove the dependency from the SR driver code
> to the voltage layer by querying the data tables only from the SR device
> init code.
> 
> Based on Paul's original code for the SmartReflex driver conversion.
> 
> Signed-off-by: Jean Pihet <j-pihet@ti.com>
> Signed-off-by: J Keerthy <j-keerthy@ti.com>
> Cc: Paul Walmsley <paul@pwsan.com>
> Cc: Thara Gopinath <thara@ti.com>
> Cc: Nishanth Menon <nm@ti.com>
> Cc: Kevin Hilman <khilman@ti.com>
> ---
>  arch/arm/mach-omap2/smartreflex.c |   38 +++++++++++++++++---------------
> ----
>  arch/arm/mach-omap2/sr_device.c   |   36 +++++++++++++++++++++++++++++---
> --
>  include/linux/power/smartreflex.h |    8 +++++-
>  3 files changed, 54 insertions(+), 28 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/smartreflex.c b/arch/arm/mach-
> omap2/smartreflex.c
> index acef08d..20075de 100644
> --- a/arch/arm/mach-omap2/smartreflex.c
> +++ b/arch/arm/mach-omap2/smartreflex.c
> @@ -347,22 +347,23 @@ static void sr_v2_disable(struct omap_sr *sr)
>  	sr_write_reg(sr, IRQSTATUS, IRQSTATUS_MCUDISABLEACKINT);
>  }
> 
> -static u32 sr_retrieve_nvalue(struct omap_sr *sr, u32 efuse_offs)
> +static struct omap_sr_nvalue_table *sr_retrieve_nvalue_row(
> +				struct omap_sr *sr, u32 efuse_offs)
>  {
>  	int i;
> 
>  	if (!sr->nvalue_table) {
>  		dev_warn(&sr->pdev->dev, "%s: Missing ntarget value table\n",
>  			__func__);
> -		return 0;
> +		return NULL;
>  	}
> 
>  	for (i = 0; i < sr->nvalue_count; i++) {
>  		if (sr->nvalue_table[i].efuse_offs == efuse_offs)
> -			return sr->nvalue_table[i].nvalue;
> +			return &sr->nvalue_table[i];
>  	}
> 
> -	return 0;
> +	return NULL;
>  }
> 
>  /* Public Functions */
> @@ -586,7 +587,7 @@ int sr_enable(struct voltagedomain *voltdm, unsigned
> long volt)
>  {
>  	struct omap_volt_data *volt_data;
>  	struct omap_sr *sr = _sr_lookup(voltdm);
> -	u32 nvalue_reciprocal;
> +	struct omap_sr_nvalue_table *nvalue_row;
>  	int ret;
> 
>  	if (IS_ERR(sr)) {
> @@ -602,16 +603,16 @@ int sr_enable(struct voltagedomain *voltdm, unsigned
> long volt)
>  		return PTR_ERR(volt_data);
>  	}
> 
> -	nvalue_reciprocal = sr_retrieve_nvalue(sr, volt_data-
> >sr_efuse_offs);
> +	nvalue_row = sr_retrieve_nvalue_row(sr, volt_data->sr_efuse_offs);
> 
> -	if (!nvalue_reciprocal) {
> -		dev_warn(&sr->pdev->dev, "%s: NVALUE = 0 at voltage %ld\n",
> -			__func__, volt);
> +	if (!nvalue_row) {
> +		dev_warn(&sr->pdev->dev, "%s: failure getting SR data for this
> voltage %ld\n",
> +			 __func__, volt);
>  		return -ENODATA;
>  	}
> 
>  	/* errminlimit is opp dependent and hence linked to voltage */
> -	sr->err_minlimit = volt_data->sr_errminlimit;
> +	sr->err_minlimit = nvalue_row->errminlimit;
> 
>  	pm_runtime_get_sync(&sr->pdev->dev);
> 
> @@ -624,7 +625,7 @@ int sr_enable(struct voltagedomain *voltdm, unsigned
> long volt)
>  	if (ret)
>  		return ret;
> 
> -	sr_write_reg(sr, NVALUERECIPROCAL, nvalue_reciprocal);
> +	sr_write_reg(sr, NVALUERECIPROCAL, nvalue_row->nvalue);
> 
>  	/* SRCONFIG - enable SR */
>  	sr_modify_reg(sr, SRCONFIG, SRCONFIG_SRENABLE, SRCONFIG_SRENABLE);
> @@ -872,7 +873,6 @@ static int __init omap_sr_probe(struct platform_device
> *pdev)
>  	struct omap_sr_data *pdata = pdev->dev.platform_data;
>  	struct resource *mem, *irq;
>  	struct dentry *nvalue_dir;
> -	struct omap_volt_data *volt_data;
>  	int i, ret = 0;
> 
>  	sr_info = kzalloc(sizeof(struct omap_sr), GFP_KERNEL);
> @@ -990,12 +990,10 @@ static int __init omap_sr_probe(struct
> platform_device *pdev)
>  		goto err_debugfs;
>  	}
> 
> -	omap_voltage_get_volttable(sr_info->voltdm, &volt_data);
> -	if (!volt_data) {
> -		dev_warn(&pdev->dev, "%s: %s: No Voltage table for the"
> -			" corresponding vdd. Cannot create debugfs"
> -			"entries for n-values\n",
> -			__func__, sr_info->name);
> +	if (sr_info->nvalue_count == 0 || !sr_info->nvalue_table) {
> +		dev_warn(&pdev->dev, "%s: %s: No Voltage table for the
> corresponding vdd. Cannot create debugfs entries for n-values\n",
> +			 __func__, sr_info->name);
> +
>  		ret = -ENODATA;
>  		goto err_debugfs;
>  	}
> @@ -1003,8 +1001,8 @@ static int __init omap_sr_probe(struct
> platform_device *pdev)
>  	for (i = 0; i < sr_info->nvalue_count; i++) {
>  		char name[NVALUE_NAME_LEN + 1];
> 
> -		snprintf(name, sizeof(name), "volt_%d",
> -			 volt_data[i].volt_nominal);
> +		snprintf(name, sizeof(name), "volt_%lu",
> +				sr_info->nvalue_table[i].volt_nominal);
>  		(void) debugfs_create_x32(name, S_IRUGO | S_IWUSR, nvalue_dir,
>  				&(sr_info->nvalue_table[i].nvalue));
>  	}
> diff --git a/arch/arm/mach-omap2/sr_device.c b/arch/arm/mach-
> omap2/sr_device.c
> index e081174..e107e39 100644
> --- a/arch/arm/mach-omap2/sr_device.c
> +++ b/arch/arm/mach-omap2/sr_device.c
> @@ -36,7 +36,10 @@ static void __init sr_set_nvalues(struct omap_volt_data
> *volt_data,
>  				struct omap_sr_data *sr_data)
>  {
>  	struct omap_sr_nvalue_table *nvalue_table;
> -	int i, count = 0;
> +	int i, j, count = 0;
> +
> +	sr_data->nvalue_count = 0;
> +	sr_data->nvalue_table = NULL;
> 
>  	while (volt_data[count].volt_nominal)
>  		count++;
> @@ -44,8 +47,14 @@ static void __init sr_set_nvalues(struct omap_volt_data
> *volt_data,
>  	nvalue_table = kzalloc(sizeof(struct omap_sr_nvalue_table)*count,
>  			GFP_KERNEL);
> 
> -	for (i = 0; i < count; i++) {
> +	if (!nvalue_table) {
> +		pr_err("OMAP: SmartReflex: cannot allocate memory for n-value
> table\n");
> +		return;
> +	}
> +
> +	for (i = 0, j = 0; i < count; i++) {
>  		u32 v;
> +
>  		/*
>  		 * In OMAP4 the efuse registers are 24 bit aligned.
>  		 * A __raw_readl will fail for non-32 bit aligned address
> @@ -58,15 +67,30 @@ static void __init sr_set_nvalues(struct
> omap_volt_data *volt_data,
>  				omap_ctrl_readb(offset + 1) << 8 |
>  				omap_ctrl_readb(offset + 2) << 16;
>  		} else {
> -			 v = omap_ctrl_readl(volt_data[i].sr_efuse_offs);
> +			v = omap_ctrl_readl(volt_data[i].sr_efuse_offs);
>  		}
> 
> -		nvalue_table[i].efuse_offs = volt_data[i].sr_efuse_offs;
> -		nvalue_table[i].nvalue = v;
> +		/*
> +		 * Many OMAP SoCs don't have the eFuse values set.
> +		 * For example, pretty much all OMAP3xxx before
> +		 * ES3.something.
> +		 *
> +		 * XXX There needs to be some way for board files or
> +		 * userspace to add these in.
> +		 */
> +		if (v == 0)
> +			continue;
> +
> +		nvalue_table[j].nvalue = v;
> +		nvalue_table[j].efuse_offs = volt_data[i].sr_efuse_offs;
> +		nvalue_table[j].errminlimit = volt_data[i].sr_errminlimit;
> +		nvalue_table[j].volt_nominal = volt_data[i].volt_nominal;
> +
> +		j++;
>  	}
> 
>  	sr_data->nvalue_table = nvalue_table;
> -	sr_data->nvalue_count = count;
> +	sr_data->nvalue_count = j;
>  }
> 
>  static int __init sr_dev_init(struct omap_hwmod *oh, void *user)
> diff --git a/include/linux/power/smartreflex.h
> b/include/linux/power/smartreflex.h
> index 78b795e..222f901 100644
> --- a/include/linux/power/smartreflex.h
> +++ b/include/linux/power/smartreflex.h
> @@ -243,12 +243,16 @@ struct omap_sr_class_data {
>  /**
>   * struct omap_sr_nvalue_table	- Smartreflex n-target value info
>   *
> - * @efuse_offs:	The offset of the efuse where n-target values are stored.
> - * @nvalue:	The n-target value.
> + * @efuse_offs:	  The offset of the efuse where n-target values are
> stored.
> + * @nvalue:	  The n-target value.
> + * @errminlimit:  The value of the ERRMINLIMIT bitfield for this n-target
> + * @volt_nominal: microvolts DC that the VDD is initially programmed to
>   */
>  struct omap_sr_nvalue_table {
>  	u32 efuse_offs;
>  	u32 nvalue;
> +	u32 errminlimit;
> +	unsigned long volt_nominal;

[GG] I would suggest that we also need to add errmaxlimit and errweight fields to this structure.  These are very much possible to change per OPP, though it hasn't been common in implementations up to now.

>  };
> 
>  /**
> --
> 1.7.5.4
> 
> --
> 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

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

* Re: [PATCH V3 07/10] ARM: OMAP2+: SmartReflex: Use per-OPP data structure
  2012-05-10 19:11   ` Guyotte, Greg
@ 2012-05-11  3:51     ` J, KEERTHY
  0 siblings, 0 replies; 49+ messages in thread
From: J, KEERTHY @ 2012-05-11  3:51 UTC (permalink / raw)
  To: Guyotte, Greg
  Cc: linux-omap, linux-arm-kernel, Hilman, Kevin, rjw, linux-kernel,
	linux-pm, Pihet-XID, Jean, Paul Walmsley, Gopinath, Thara, Menon,
	Nishanth

Hi Greg,

Thanks for reviewing.

On Fri, May 11, 2012 at 12:41 AM, Guyotte, Greg <gguyotte@ti.com> wrote:
> Hi Jean,
>
>> -----Original Message-----
>> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
>> owner@vger.kernel.org] On Behalf Of J, KEERTHY
>> Sent: Thursday, April 26, 2012 12:41 PM
>> To: linux-omap@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
>> Hilman, Kevin; rjw@sisk.pl; linux-kernel@vger.kernel.org; linux-
>> pm@lists.linux-foundation.org
>> Cc: Pihet-XID, Jean; J, KEERTHY; Paul Walmsley; Gopinath, Thara; Menon,
>> Nishanth
>> Subject: [PATCH V3 07/10] ARM: OMAP2+: SmartReflex: Use per-OPP data
>> structure
>>
>> From: Jean Pihet <j-pihet@ti.com>
>>
>> The SmartReflex driver incorrectly treats some per-OPP data as data
>> common to all OPPs (e.g., ERRMINLIMIT).  Move this data into a per-OPP
>> data structure.
>>
>> Furthermore, in order to make the SmartReflex implementation ready for
>> the move to drivers/, remove the dependency from the SR driver code
>> to the voltage layer by querying the data tables only from the SR device
>> init code.
>>
>> Based on Paul's original code for the SmartReflex driver conversion.
>>
>> Signed-off-by: Jean Pihet <j-pihet@ti.com>
>> Signed-off-by: J Keerthy <j-keerthy@ti.com>
>> Cc: Paul Walmsley <paul@pwsan.com>
>> Cc: Thara Gopinath <thara@ti.com>
>> Cc: Nishanth Menon <nm@ti.com>
>> Cc: Kevin Hilman <khilman@ti.com>
>> ---
>>  arch/arm/mach-omap2/smartreflex.c |   38 +++++++++++++++++---------------
>> ----
>>  arch/arm/mach-omap2/sr_device.c   |   36 +++++++++++++++++++++++++++++---
>> --
>>  include/linux/power/smartreflex.h |    8 +++++-
>>  3 files changed, 54 insertions(+), 28 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/smartreflex.c b/arch/arm/mach-
>> omap2/smartreflex.c
>> index acef08d..20075de 100644
>> --- a/arch/arm/mach-omap2/smartreflex.c
>> +++ b/arch/arm/mach-omap2/smartreflex.c
>> @@ -347,22 +347,23 @@ static void sr_v2_disable(struct omap_sr *sr)
>>       sr_write_reg(sr, IRQSTATUS, IRQSTATUS_MCUDISABLEACKINT);
>>  }
>>
>> -static u32 sr_retrieve_nvalue(struct omap_sr *sr, u32 efuse_offs)
>> +static struct omap_sr_nvalue_table *sr_retrieve_nvalue_row(
>> +                             struct omap_sr *sr, u32 efuse_offs)
>>  {
>>       int i;
>>
>>       if (!sr->nvalue_table) {
>>               dev_warn(&sr->pdev->dev, "%s: Missing ntarget value table\n",
>>                       __func__);
>> -             return 0;
>> +             return NULL;
>>       }
>>
>>       for (i = 0; i < sr->nvalue_count; i++) {
>>               if (sr->nvalue_table[i].efuse_offs == efuse_offs)
>> -                     return sr->nvalue_table[i].nvalue;
>> +                     return &sr->nvalue_table[i];
>>       }
>>
>> -     return 0;
>> +     return NULL;
>>  }
>>
>>  /* Public Functions */
>> @@ -586,7 +587,7 @@ int sr_enable(struct voltagedomain *voltdm, unsigned
>> long volt)
>>  {
>>       struct omap_volt_data *volt_data;
>>       struct omap_sr *sr = _sr_lookup(voltdm);
>> -     u32 nvalue_reciprocal;
>> +     struct omap_sr_nvalue_table *nvalue_row;
>>       int ret;
>>
>>       if (IS_ERR(sr)) {
>> @@ -602,16 +603,16 @@ int sr_enable(struct voltagedomain *voltdm, unsigned
>> long volt)
>>               return PTR_ERR(volt_data);
>>       }
>>
>> -     nvalue_reciprocal = sr_retrieve_nvalue(sr, volt_data-
>> >sr_efuse_offs);
>> +     nvalue_row = sr_retrieve_nvalue_row(sr, volt_data->sr_efuse_offs);
>>
>> -     if (!nvalue_reciprocal) {
>> -             dev_warn(&sr->pdev->dev, "%s: NVALUE = 0 at voltage %ld\n",
>> -                     __func__, volt);
>> +     if (!nvalue_row) {
>> +             dev_warn(&sr->pdev->dev, "%s: failure getting SR data for this
>> voltage %ld\n",
>> +                      __func__, volt);
>>               return -ENODATA;
>>       }
>>
>>       /* errminlimit is opp dependent and hence linked to voltage */
>> -     sr->err_minlimit = volt_data->sr_errminlimit;
>> +     sr->err_minlimit = nvalue_row->errminlimit;
>>
>>       pm_runtime_get_sync(&sr->pdev->dev);
>>
>> @@ -624,7 +625,7 @@ int sr_enable(struct voltagedomain *voltdm, unsigned
>> long volt)
>>       if (ret)
>>               return ret;
>>
>> -     sr_write_reg(sr, NVALUERECIPROCAL, nvalue_reciprocal);
>> +     sr_write_reg(sr, NVALUERECIPROCAL, nvalue_row->nvalue);
>>
>>       /* SRCONFIG - enable SR */
>>       sr_modify_reg(sr, SRCONFIG, SRCONFIG_SRENABLE, SRCONFIG_SRENABLE);
>> @@ -872,7 +873,6 @@ static int __init omap_sr_probe(struct platform_device
>> *pdev)
>>       struct omap_sr_data *pdata = pdev->dev.platform_data;
>>       struct resource *mem, *irq;
>>       struct dentry *nvalue_dir;
>> -     struct omap_volt_data *volt_data;
>>       int i, ret = 0;
>>
>>       sr_info = kzalloc(sizeof(struct omap_sr), GFP_KERNEL);
>> @@ -990,12 +990,10 @@ static int __init omap_sr_probe(struct
>> platform_device *pdev)
>>               goto err_debugfs;
>>       }
>>
>> -     omap_voltage_get_volttable(sr_info->voltdm, &volt_data);
>> -     if (!volt_data) {
>> -             dev_warn(&pdev->dev, "%s: %s: No Voltage table for the"
>> -                     " corresponding vdd. Cannot create debugfs"
>> -                     "entries for n-values\n",
>> -                     __func__, sr_info->name);
>> +     if (sr_info->nvalue_count == 0 || !sr_info->nvalue_table) {
>> +             dev_warn(&pdev->dev, "%s: %s: No Voltage table for the
>> corresponding vdd. Cannot create debugfs entries for n-values\n",
>> +                      __func__, sr_info->name);
>> +
>>               ret = -ENODATA;
>>               goto err_debugfs;
>>       }
>> @@ -1003,8 +1001,8 @@ static int __init omap_sr_probe(struct
>> platform_device *pdev)
>>       for (i = 0; i < sr_info->nvalue_count; i++) {
>>               char name[NVALUE_NAME_LEN + 1];
>>
>> -             snprintf(name, sizeof(name), "volt_%d",
>> -                      volt_data[i].volt_nominal);
>> +             snprintf(name, sizeof(name), "volt_%lu",
>> +                             sr_info->nvalue_table[i].volt_nominal);
>>               (void) debugfs_create_x32(name, S_IRUGO | S_IWUSR, nvalue_dir,
>>                               &(sr_info->nvalue_table[i].nvalue));
>>       }
>> diff --git a/arch/arm/mach-omap2/sr_device.c b/arch/arm/mach-
>> omap2/sr_device.c
>> index e081174..e107e39 100644
>> --- a/arch/arm/mach-omap2/sr_device.c
>> +++ b/arch/arm/mach-omap2/sr_device.c
>> @@ -36,7 +36,10 @@ static void __init sr_set_nvalues(struct omap_volt_data
>> *volt_data,
>>                               struct omap_sr_data *sr_data)
>>  {
>>       struct omap_sr_nvalue_table *nvalue_table;
>> -     int i, count = 0;
>> +     int i, j, count = 0;
>> +
>> +     sr_data->nvalue_count = 0;
>> +     sr_data->nvalue_table = NULL;
>>
>>       while (volt_data[count].volt_nominal)
>>               count++;
>> @@ -44,8 +47,14 @@ static void __init sr_set_nvalues(struct omap_volt_data
>> *volt_data,
>>       nvalue_table = kzalloc(sizeof(struct omap_sr_nvalue_table)*count,
>>                       GFP_KERNEL);
>>
>> -     for (i = 0; i < count; i++) {
>> +     if (!nvalue_table) {
>> +             pr_err("OMAP: SmartReflex: cannot allocate memory for n-value
>> table\n");
>> +             return;
>> +     }
>> +
>> +     for (i = 0, j = 0; i < count; i++) {
>>               u32 v;
>> +
>>               /*
>>                * In OMAP4 the efuse registers are 24 bit aligned.
>>                * A __raw_readl will fail for non-32 bit aligned address
>> @@ -58,15 +67,30 @@ static void __init sr_set_nvalues(struct
>> omap_volt_data *volt_data,
>>                               omap_ctrl_readb(offset + 1) << 8 |
>>                               omap_ctrl_readb(offset + 2) << 16;
>>               } else {
>> -                      v = omap_ctrl_readl(volt_data[i].sr_efuse_offs);
>> +                     v = omap_ctrl_readl(volt_data[i].sr_efuse_offs);
>>               }
>>
>> -             nvalue_table[i].efuse_offs = volt_data[i].sr_efuse_offs;
>> -             nvalue_table[i].nvalue = v;
>> +             /*
>> +              * Many OMAP SoCs don't have the eFuse values set.
>> +              * For example, pretty much all OMAP3xxx before
>> +              * ES3.something.
>> +              *
>> +              * XXX There needs to be some way for board files or
>> +              * userspace to add these in.
>> +              */
>> +             if (v == 0)
>> +                     continue;
>> +
>> +             nvalue_table[j].nvalue = v;
>> +             nvalue_table[j].efuse_offs = volt_data[i].sr_efuse_offs;
>> +             nvalue_table[j].errminlimit = volt_data[i].sr_errminlimit;
>> +             nvalue_table[j].volt_nominal = volt_data[i].volt_nominal;
>> +
>> +             j++;
>>       }
>>
>>       sr_data->nvalue_table = nvalue_table;
>> -     sr_data->nvalue_count = count;
>> +     sr_data->nvalue_count = j;
>>  }
>>
>>  static int __init sr_dev_init(struct omap_hwmod *oh, void *user)
>> diff --git a/include/linux/power/smartreflex.h
>> b/include/linux/power/smartreflex.h
>> index 78b795e..222f901 100644
>> --- a/include/linux/power/smartreflex.h
>> +++ b/include/linux/power/smartreflex.h
>> @@ -243,12 +243,16 @@ struct omap_sr_class_data {
>>  /**
>>   * struct omap_sr_nvalue_table       - Smartreflex n-target value info
>>   *
>> - * @efuse_offs:      The offset of the efuse where n-target values are stored.
>> - * @nvalue:  The n-target value.
>> + * @efuse_offs:        The offset of the efuse where n-target values are
>> stored.
>> + * @nvalue:    The n-target value.
>> + * @errminlimit:  The value of the ERRMINLIMIT bitfield for this n-target
>> + * @volt_nominal: microvolts DC that the VDD is initially programmed to
>>   */
>>  struct omap_sr_nvalue_table {
>>       u32 efuse_offs;
>>       u32 nvalue;
>> +     u32 errminlimit;
>> +     unsigned long volt_nominal;
>
> [GG] I would suggest that we also need to add errmaxlimit and errweight fields to this structure.  These are very much possible to change per OPP, though it hasn't been common in implementations up to now.

Ok. Since this patch set is focussed on minimal clean up and moving the code
from mach-omap to drivers this is not addressed. I will sure take this up once
this is moved.

>
>>  };
>>
>>  /**
>> --
>> 1.7.5.4
>>
>> --
>> 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



-- 
Regards and Thanks,
Keerthy

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

* Re: [PATCH V3 00/10] PM: Create the AVS(Adaptive Voltage Scaling)
  2012-05-07 23:51 ` Kevin Hilman
@ 2012-05-15  5:46   ` J, KEERTHY
  2012-05-23  4:51     ` J, KEERTHY
  0 siblings, 1 reply; 49+ messages in thread
From: J, KEERTHY @ 2012-05-15  5:46 UTC (permalink / raw)
  To: Rafael J. Wysocki, Kevin Hilman
  Cc: linux-omap, linux-arm-kernel, linux-kernel, linux-pm, j-pihet

On Tue, May 8, 2012 at 5:21 AM, Kevin Hilman <khilman@ti.com> wrote:
> Rafael,
>
> Keerthy <j-keerthy@ti.com> writes:
>
>> From: J Keerthy <j-keerthy@ti.com>
>>
>> AVS(Adaptive Voltage Scaling) is a power management technique which
>> controls the operating voltage of a device in order to optimize (i.e. reduce)
>> its power consumption. The voltage is adapted depending on static factors
>> (chip manufacturing process) and dynamic factors (temperature
>> depending performance).
>> The TI AVS solution is named Smartreflex.
>>
>> To that end, create the AVS driver in drivers/power/avs and
>> move the OMAP SmartReflex code to the new directory. The
>> class driver is still retained in the mach-omap2 directory.
>
> How should we handle this for upstream?
>
> It does a bunch of cleanup under arch/arm then does the move to
> drivers/power the end.  To avoid conflicts with other OMAP core changes,
> I would suggest we take this through the OMAP tree.
>
> With your ack, I'd be glad to take it.

Hello Rafael,

A gentle ping on this series.

>
> Thanks,
>
> Kevin
>



-- 
Regards and Thanks,
Keerthy

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

* Re: [PATCH V3 00/10] PM: Create the AVS(Adaptive Voltage Scaling)
  2012-05-15  5:46   ` J, KEERTHY
@ 2012-05-23  4:51     ` J, KEERTHY
  2012-05-24 17:24       ` Kevin Hilman
  0 siblings, 1 reply; 49+ messages in thread
From: J, KEERTHY @ 2012-05-23  4:51 UTC (permalink / raw)
  To: Rafael J. Wysocki, Kevin Hilman, greg
  Cc: linux-omap, linux-arm-kernel, linux-kernel, linux-pm, j-pihet

On Tue, May 15, 2012 at 11:16 AM, J, KEERTHY <j-keerthy@ti.com> wrote:
> On Tue, May 8, 2012 at 5:21 AM, Kevin Hilman <khilman@ti.com> wrote:
>> Rafael,
>>
>> Keerthy <j-keerthy@ti.com> writes:
>>
>>> From: J Keerthy <j-keerthy@ti.com>
>>>
>>> AVS(Adaptive Voltage Scaling) is a power management technique which
>>> controls the operating voltage of a device in order to optimize (i.e. reduce)
>>> its power consumption. The voltage is adapted depending on static factors
>>> (chip manufacturing process) and dynamic factors (temperature
>>> depending performance).
>>> The TI AVS solution is named Smartreflex.
>>>
>>> To that end, create the AVS driver in drivers/power/avs and
>>> move the OMAP SmartReflex code to the new directory. The
>>> class driver is still retained in the mach-omap2 directory.
>>
>> How should we handle this for upstream?
>>
>> It does a bunch of cleanup under arch/arm then does the move to
>> drivers/power the end.  To avoid conflicts with other OMAP core changes,
>> I would suggest we take this through the OMAP tree.
>>
>> With your ack, I'd be glad to take it.
>
> Hello Rafael,
>
> A gentle ping on this series.

Hi Greg,

This series has Kevin's comments incorporated.

Kevin,

Can i have your Ack for this series?

>
>>
>> Thanks,
>>
>> Kevin
>>
>
>
>
> --
> Regards and Thanks,
> Keerthy



-- 
Regards and Thanks,
Keerthy

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

* Re: [linux-pm] [PATCH V3 00/10] PM: Create the AVS(Adaptive Voltage Scaling)
  2012-05-09 18:29                     ` Kevin Hilman
@ 2012-05-23 13:27                       ` Menon, Nishanth
  2012-05-24 23:16                         ` Kevin Hilman
  0 siblings, 1 reply; 49+ messages in thread
From: Menon, Nishanth @ 2012-05-23 13:27 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: Woodruff, Richard, J, KEERTHY, Mark Brown, linux-kernel,
	AnilKumar, Chimata, linux-pm, linux-omap, Pihet-XID, Jean,
	linux-arm-kernel

On Wed, May 9, 2012 at 1:29 PM, Kevin Hilman <khilman@ti.com> wrote:
> "Woodruff, Richard" <r-woodruff2@ti.com> writes:
>
>>> From: Hilman, Kevin
>>> Sent: Tuesday, May 08, 2012 5:17 PM
>>
>>> A basic OMAP AVS driver has been in mainline for a long time, yet we
>>> have not seen support submitted for all of these features.
>>
>> 1.5/3.5 is a feature.
>
> And I'm still waiting for it to be submitted upstream.
>
>> ABB is requirement for a production useable driver. Higher speed rated
>> OMAP4 and all OMAP5 added these to be useable.
>
> ditto
>
>> Yes this is effort. Point of mentioning is to raise awareness of need.
>
> I'm well aware of the need.
>
>> Yet to be added feature has different meaning than functional gap.
>
> And both need to be submitted upstream.

SR 1.5: http://marc.info/?l=linux-omap&m=129933897910785&w=2
ABB: http://marc.info/?l=linux-omap&m=130939399209099&w=2

I am not sure what you mean "need to be submitted upstream"?

Just tired of seeing things perpetually change without considering
even how to handle features that are mandatory for SoC even with code
posted upstream to show exactly what it takes.. I think you do mean
merged upstream in this context.


Regards,
Nishanth Menon

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

* Re: [PATCH V3 00/10] PM: Create the AVS(Adaptive Voltage Scaling)
  2012-05-23  4:51     ` J, KEERTHY
@ 2012-05-24 17:24       ` Kevin Hilman
  2012-05-31 22:40         ` Kevin Hilman
  0 siblings, 1 reply; 49+ messages in thread
From: Kevin Hilman @ 2012-05-24 17:24 UTC (permalink / raw)
  To: J, KEERTHY
  Cc: Rafael J. Wysocki, greg, linux-omap, linux-arm-kernel,
	linux-kernel, linux-pm, j-pihet

"J, KEERTHY" <j-keerthy@ti.com> writes:

> On Tue, May 15, 2012 at 11:16 AM, J, KEERTHY <j-keerthy@ti.com> wrote:
>> On Tue, May 8, 2012 at 5:21 AM, Kevin Hilman <khilman@ti.com> wrote:
>>> Rafael,
>>>
>>> Keerthy <j-keerthy@ti.com> writes:
>>>
>>>> From: J Keerthy <j-keerthy@ti.com>
>>>>
>>>> AVS(Adaptive Voltage Scaling) is a power management technique which
>>>> controls the operating voltage of a device in order to optimize (i.e. reduce)
>>>> its power consumption. The voltage is adapted depending on static factors
>>>> (chip manufacturing process) and dynamic factors (temperature
>>>> depending performance).
>>>> The TI AVS solution is named Smartreflex.
>>>>
>>>> To that end, create the AVS driver in drivers/power/avs and
>>>> move the OMAP SmartReflex code to the new directory. The
>>>> class driver is still retained in the mach-omap2 directory.
>>>
>>> How should we handle this for upstream?
>>>
>>> It does a bunch of cleanup under arch/arm then does the move to
>>> drivers/power the end.  To avoid conflicts with other OMAP core changes,
>>> I would suggest we take this through the OMAP tree.
>>>
>>> With your ack, I'd be glad to take it.
>>
>> Hello Rafael,
>>
>> A gentle ping on this series.
>
> Hi Greg,
>
> This series has Kevin's comments incorporated.
>
> Kevin,
>
> Can i have your Ack for this series?
>

Well, as mentioned above, I'm waiting for Rafael's ack, then I will
merge it.

Because of all the arch/arm/mach-omap2/* changes, I would like to merge
this via the OMAP tree to avoid conflicts with other stuff we have
changing in arch/arm/mach-omap2/*

Kevin


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

* Re: [linux-pm] [PATCH V3 00/10] PM: Create the AVS(Adaptive Voltage Scaling)
  2012-05-23 13:27                       ` Menon, Nishanth
@ 2012-05-24 23:16                         ` Kevin Hilman
  0 siblings, 0 replies; 49+ messages in thread
From: Kevin Hilman @ 2012-05-24 23:16 UTC (permalink / raw)
  To: Menon, Nishanth
  Cc: J, KEERTHY, Mark Brown, linux-kernel, AnilKumar, Chimata,
	linux-pm, linux-omap, Pihet-XID, Jean, linux-arm-kernel

"Menon, Nishanth" <nm@ti.com> writes:

> On Wed, May 9, 2012 at 1:29 PM, Kevin Hilman <khilman@ti.com> wrote:
>> "Woodruff, Richard" <r-woodruff2@ti.com> writes:
>>
>>>> From: Hilman, Kevin
>>>> Sent: Tuesday, May 08, 2012 5:17 PM
>>>
>>>> A basic OMAP AVS driver has been in mainline for a long time, yet we
>>>> have not seen support submitted for all of these features.
>>>
>>> 1.5/3.5 is a feature.
>>
>> And I'm still waiting for it to be submitted upstream.
>>
>>> ABB is requirement for a production useable driver. Higher speed rated
>>> OMAP4 and all OMAP5 added these to be useable.
>>
>> ditto
>>
>>> Yes this is effort. Point of mentioning is to raise awareness of need.
>>
>> I'm well aware of the need.
>>
>>> Yet to be added feature has different meaning than functional gap.
>>
>> And both need to be submitted upstream.
>
> SR 1.5: http://marc.info/?l=linux-omap&m=129933897910785&w=2
> ABB: http://marc.info/?l=linux-omap&m=130939399209099&w=2
>
> I am not sure what you mean "need to be submitted upstream"?

You're right.  I should've said re-submitted and merged.  Both have been
submitted (and reviewed) but no follow up submissions after review, and
thus they're still out of tree.

> Just tired of seeing things perpetually change without considering
> even how to handle features that are mandatory for SoC even with code
> posted upstream to show exactly what it takes.. 

I'm sorry, but this is not perpetual change.  

This driver has been upstream in its current (admittedly
feature-limited) form for a long time, the only thing changing in
$SUBJECT series is the location of the driver.  Why all the fuss about
the missing features now?

> I think you do mean merged upstream in this context.

Correct.

Frameworks always have limitations.  The way they get extended/expanded
etc. is by the submission/review/merging of support for new
features/requirements.  The process for that is the same as any feature
in any part of the kernel.

Evolution, not intelligent design[1].

All of that being said, I'm not sure why this thread was hijacked for
this debate in the first place.  The point of $SUBJECT series is simply
to move and *existing* framework from arch/arm out to drivers.  The only
changes done are cleanups to make this move possible.

I for one would welcome extending this framework to ensure it supports
all the SoC features.  I just don't want those features to be a
prerequisite for this move from arch/arm to drivers.

Please, let's get this moved to drivers, and then add support for the
missing features.

Thanks,

Kevin

[1] http://kerneltrap.org/Linux/Kernel_Evolution

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

* Re: [PATCH V3 00/10] PM: Create the AVS(Adaptive Voltage Scaling)
  2012-05-24 17:24       ` Kevin Hilman
@ 2012-05-31 22:40         ` Kevin Hilman
  2012-06-01  3:45           ` J, KEERTHY
  0 siblings, 1 reply; 49+ messages in thread
From: Kevin Hilman @ 2012-05-31 22:40 UTC (permalink / raw)
  To: J, KEERTHY
  Cc: Rafael J. Wysocki, greg, linux-omap, linux-arm-kernel,
	linux-kernel, linux-pm, j-pihet

Kevin Hilman <khilman@ti.com> writes:

> "J, KEERTHY" <j-keerthy@ti.com> writes:
>
>> On Tue, May 15, 2012 at 11:16 AM, J, KEERTHY <j-keerthy@ti.com> wrote:
>>> On Tue, May 8, 2012 at 5:21 AM, Kevin Hilman <khilman@ti.com> wrote:
>>>> Rafael,
>>>>
>>>> Keerthy <j-keerthy@ti.com> writes:
>>>>
>>>>> From: J Keerthy <j-keerthy@ti.com>
>>>>>
>>>>> AVS(Adaptive Voltage Scaling) is a power management technique which
>>>>> controls the operating voltage of a device in order to optimize (i.e. reduce)
>>>>> its power consumption. The voltage is adapted depending on static factors
>>>>> (chip manufacturing process) and dynamic factors (temperature
>>>>> depending performance).
>>>>> The TI AVS solution is named Smartreflex.
>>>>>
>>>>> To that end, create the AVS driver in drivers/power/avs and
>>>>> move the OMAP SmartReflex code to the new directory. The
>>>>> class driver is still retained in the mach-omap2 directory.
>>>>
>>>> How should we handle this for upstream?
>>>>
>>>> It does a bunch of cleanup under arch/arm then does the move to
>>>> drivers/power the end.  To avoid conflicts with other OMAP core changes,
>>>> I would suggest we take this through the OMAP tree.
>>>>
>>>> With your ack, I'd be glad to take it.
>>>
>>> Hello Rafael,
>>>
>>> A gentle ping on this series.
>>
>> Hi Greg,
>>
>> This series has Kevin's comments incorporated.
>>
>> Kevin,
>>
>> Can i have your Ack for this series?
>>
>
> Well, as mentioned above, I'm waiting for Rafael's ack, then I will
> merge it.
>
> Because of all the arch/arm/mach-omap2/* changes, I would like to merge
> this via the OMAP tree to avoid conflicts with other stuff we have
> changing in arch/arm/mach-omap2/*

OK, I had an off-line discussion with Rafael and he's OK that I take
these.  I will add an ack from Rafael and queue this series up for v3.6.

Thanks,

Kevin

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

* Re: [PATCH V3 00/10] PM: Create the AVS(Adaptive Voltage Scaling)
  2012-05-31 22:40         ` Kevin Hilman
@ 2012-06-01  3:45           ` J, KEERTHY
  0 siblings, 0 replies; 49+ messages in thread
From: J, KEERTHY @ 2012-06-01  3:45 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: Rafael J. Wysocki, greg, linux-omap, linux-arm-kernel,
	linux-kernel, linux-pm, j-pihet

On Fri, Jun 1, 2012 at 4:10 AM, Kevin Hilman <khilman@ti.com> wrote:
> Kevin Hilman <khilman@ti.com> writes:
>
>> "J, KEERTHY" <j-keerthy@ti.com> writes:
>>
>>> On Tue, May 15, 2012 at 11:16 AM, J, KEERTHY <j-keerthy@ti.com> wrote:
>>>> On Tue, May 8, 2012 at 5:21 AM, Kevin Hilman <khilman@ti.com> wrote:
>>>>> Rafael,
>>>>>
>>>>> Keerthy <j-keerthy@ti.com> writes:
>>>>>
>>>>>> From: J Keerthy <j-keerthy@ti.com>
>>>>>>
>>>>>> AVS(Adaptive Voltage Scaling) is a power management technique which
>>>>>> controls the operating voltage of a device in order to optimize (i.e. reduce)
>>>>>> its power consumption. The voltage is adapted depending on static factors
>>>>>> (chip manufacturing process) and dynamic factors (temperature
>>>>>> depending performance).
>>>>>> The TI AVS solution is named Smartreflex.
>>>>>>
>>>>>> To that end, create the AVS driver in drivers/power/avs and
>>>>>> move the OMAP SmartReflex code to the new directory. The
>>>>>> class driver is still retained in the mach-omap2 directory.
>>>>>
>>>>> How should we handle this for upstream?
>>>>>
>>>>> It does a bunch of cleanup under arch/arm then does the move to
>>>>> drivers/power the end.  To avoid conflicts with other OMAP core changes,
>>>>> I would suggest we take this through the OMAP tree.
>>>>>
>>>>> With your ack, I'd be glad to take it.
>>>>
>>>> Hello Rafael,
>>>>
>>>> A gentle ping on this series.
>>>
>>> Hi Greg,
>>>
>>> This series has Kevin's comments incorporated.
>>>
>>> Kevin,
>>>
>>> Can i have your Ack for this series?
>>>
>>
>> Well, as mentioned above, I'm waiting for Rafael's ack, then I will
>> merge it.
>>
>> Because of all the arch/arm/mach-omap2/* changes, I would like to merge
>> this via the OMAP tree to avoid conflicts with other stuff we have
>> changing in arch/arm/mach-omap2/*
>
> OK, I had an off-line discussion with Rafael and he's OK that I take
> these.  I will add an ack from Rafael and queue this series up for v3.6.

Thanks Kevin.

>
> Thanks,
>
> Kevin



-- 
Regards and Thanks,
Keerthy

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

* [PATCH V3 00/10] PM: Create the AVS(Adaptive Voltage Scaling)
@ 2012-04-26 18:05 Keerthy
  0 siblings, 0 replies; 49+ messages in thread
From: Keerthy @ 2012-04-26 18:05 UTC (permalink / raw)
  To: linux-omap, linux-arm-kernel, khilman, rjw, linux-kernel, linux-pm
  Cc: j-pihet, j-keerthy

From: J Keerthy <j-keerthy@ti.com>

AVS(Adaptive Voltage Scaling) is a power management technique which
controls the operating voltage of a device in order to optimize (i.e. reduce)
its power consumption. The voltage is adapted depending on static factors
(chip manufacturing process) and dynamic factors (temperature
depending performance).
The TI AVS solution is named Smartreflex. 

To that end, create the AVS driver in drivers/power/avs and
move the OMAP SmartReflex code to the new directory. The
class driver is still retained in the mach-omap2 directory.

In preparation to the move of the OMAP code the following changes have been
made:
- fill in platform data from the device initialization code and pass
 it to the driver,
- create CONFIG_AVS* config options accordingly.

The platform integration data for SmartReflex is passed from hwmod
and the voltage layer to the driver using pdata.

Tested on OMAP4430 SDP using omap2plus_defconfig with the
CONFIG_POWER_AVS* options set. Voltage correction seen
on oscilloscope on all three VDDs.
Based on master branch of the l-o git tree, commit
6bd61e8de0511dd9831eb9d89eea0b4603a10e9e.

Tree: git://gitorious.org/~keerthy05/omap-pm/keerthy-sr.git for_smartreflex_ip_driver_move 

V3: rework after the comments on MLs
 . Retain Efuse offsets check to identify a particular OPP.
 . Introduce a common header file accessible both by arch/arm/mach-omap2/
   and drivers/.
 . Retain the class driver in mach-omap2/ and move IP driver to drivers/power/
   avs since class driver needs voltage layer support.

History:
v2: rework after the comments on MLs
 . Keep the OMAP Kconfig options in the arch dir (Rafael),
 . Move the shared header file from plat-omap to
   include/linux/power/ (Tony)

v1: initial revision

J Keerthy (1):
  ARM: OMAP2+: Voltage: Move the omap_volt_data structure to plat

Jean Pihet (9):
  ARM: OMAP2+: SmartReflex: move the smartreflex header to include/linux/power
  ARM: OMAP3+: SmartReflex: class drivers should use struct omap_sr *
  ARM: OMAP2+: smartreflex: Use the names from hwmod data instead of
    voltage domains.
  ARM: OMAP3: hwmod: rename the smartreflex entries
  ARM: OMAP2+: SmartReflex: introduce a busy loop condition test macro
  ARM: OMAP2+: SmartReflex: Use per-OPP data structure
  ARM: OMAP2+: SmartReflex: Create per-opp debugfs node for errminlimit
  ARM: OMAP2+: SmartReflex: add POWER_AVS Kconfig options
  ARM: OMAP: SmartReflex: Move smartreflex driver to drivers/
 arch/arm/mach-omap2/Makefile                       |    5 +-
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c         |   12 +-
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c         |    3 +-
 arch/arm/mach-omap2/pm.h                           |    2 +-
 arch/arm/mach-omap2/smartreflex-class3.c           |   29 ++--
 arch/arm/mach-omap2/sr_device.c                    |   39 ++++-
 arch/arm/mach-omap2/voltage.h                      |   21 +---
 arch/arm/plat-omap/Kconfig                         |   31 ++--
 arch/arm/plat-omap/include/plat/voltage.h          |   21 +++-
 drivers/power/Kconfig                              |    2 +
 drivers/power/Makefile                             |    1 +
 drivers/power/avs/Kconfig                          |   12 ++
 drivers/power/avs/Makefile                         |    1 +
 .../mach-omap2 => drivers/power/avs}/smartreflex.c |  161 ++++++++------------
 .../linux/power}/smartreflex.h                     |   74 ++++++++--
 15 files changed, 235 insertions(+), 179 deletions(-)
 create mode 100644 drivers/power/avs/Kconfig
 create mode 100644 drivers/power/avs/Makefile
 rename {arch/arm/mach-omap2 => drivers/power/avs}/smartreflex.c (90%)
 rename {arch/arm/mach-omap2 => include/linux/power}/smartreflex.h (79%)

-- 
1.7.5.4


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

end of thread, other threads:[~2012-06-01  3:45 UTC | newest]

Thread overview: 49+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-26 17:40 [PATCH V3 00/10] PM: Create the AVS(Adaptive Voltage Scaling) Keerthy
2012-04-26 17:40 ` [PATCH V3 01/10] ARM: OMAP2+: SmartReflex: move the smartreflex header to include/linux/power Keerthy
2012-04-26 17:40 ` [PATCH V3 02/10] ARM: OMAP3+: SmartReflex: class drivers should use struct omap_sr * Keerthy
2012-04-26 17:40 ` [PATCH V3 03/10] ARM: OMAP2+: smartreflex: Use the names from hwmod data instead of voltage domains Keerthy
2012-04-26 17:40 ` [PATCH V3 04/10] ARM: OMAP3: hwmod: rename the smartreflex entries Keerthy
2012-05-04  8:30   ` AnilKumar, Chimata
2012-05-04 10:11     ` J, KEERTHY
2012-05-07 23:39       ` Kevin Hilman
2012-05-07 23:55         ` Kevin Hilman
2012-05-08  3:44           ` J, KEERTHY
2012-04-26 17:40 ` [PATCH V3 05/10] ARM: OMAP2+: SmartReflex: introduce a busy loop condition test macro Keerthy
2012-05-04  9:12   ` AnilKumar, Chimata
2012-05-07  5:21     ` J, KEERTHY
2012-05-08 10:17       ` AnilKumar, Chimata
2012-05-10  6:19         ` J, KEERTHY
2012-04-26 17:40 ` [PATCH V3 06/10] ARM: OMAP2+: Voltage: Move the omap_volt_data structure to plat Keerthy
2012-04-26 17:40 ` [PATCH V3 07/10] ARM: OMAP2+: SmartReflex: Use per-OPP data structure Keerthy
2012-05-10 19:11   ` Guyotte, Greg
2012-05-11  3:51     ` J, KEERTHY
2012-04-26 17:40 ` [PATCH V3 08/10] ARM: OMAP2+: SmartReflex: Create per-opp debugfs node for errminlimit Keerthy
2012-04-26 17:40 ` [PATCH V3 09/10] ARM: OMAP2+: SmartReflex: add POWER_AVS Kconfig options Keerthy
2012-04-26 17:40 ` [PATCH V3 10/10] ARM: OMAP: SmartReflex: Move smartreflex driver to drivers/ Keerthy
2012-04-26 19:11 ` [PATCH V3 00/10] PM: Create the AVS(Adaptive Voltage Scaling) Mark Brown
2012-04-27  5:39   ` J, KEERTHY
2012-04-27 17:56     ` Mark Brown
2012-04-27 21:01       ` Kevin Hilman
2012-04-30  4:25         ` J, KEERTHY
2012-04-30  9:54         ` Mark Brown
2012-04-30 21:51           ` Kevin Hilman
2012-05-02  5:04             ` J, KEERTHY
2012-05-04  5:05               ` J, KEERTHY
2012-05-04  8:21         ` AnilKumar, Chimata
2012-05-07 23:48           ` Kevin Hilman
2012-05-08  3:48             ` J, KEERTHY
2012-05-08 10:17             ` AnilKumar, Chimata
2012-05-08 20:38               ` Woodruff, Richard
2012-05-08 22:16                 ` [linux-pm] " Kevin Hilman
2012-05-09  0:39                   ` Woodruff, Richard
2012-05-09  8:19                     ` Koen Kooi
2012-05-09 18:29                     ` Kevin Hilman
2012-05-23 13:27                       ` Menon, Nishanth
2012-05-24 23:16                         ` Kevin Hilman
2012-05-07 23:51 ` Kevin Hilman
2012-05-15  5:46   ` J, KEERTHY
2012-05-23  4:51     ` J, KEERTHY
2012-05-24 17:24       ` Kevin Hilman
2012-05-31 22:40         ` Kevin Hilman
2012-06-01  3:45           ` J, KEERTHY
2012-04-26 18:05 Keerthy

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