[PATCH 00/10 v4] omap3: pm: introduce support for 3630 OPPs

[PATCH 00/10 v4] omap3: pm: introduce support for 3630 OPPs

[Nishanth Menon]

Hi,
Thanks for all the comments. Here is V4 of the patch series.
What changed in V4 (or stage1 as discussed in l-o v3):
* Search OPP APIs changed.
* review comment incorporated

History:
V3: http://marc.info/?t=125912234200001&r=1&w=2
V2: http://marc.info/?l=linux-omap&m=125809232732700&w=2
V1: http://marc.info/?l=linux-omap&m=125800488923479&w=2

Ref: http://elinux.org/OMAP_Power_Management#Future_directions

Also note that OMAP3630 is dependent on clk tree changes which needs
to come in - but that is an independent parallel activity.

NOTE: SR should be disabled for testing on 3630, there are deltas for
OMAP3630 which are not in place in SR codebase and behavior is not
trustworthy.

TODO Cleanups:
* Kevin's recommended cleanups in:
  http://marc.info/?l=linux-omap&m=125917007423545&w=2
  (requesting this be posted once we get the initial set in..)
* SRF cleanup in a detailed and to make it robust.
* Smartreflex cleanups/refactor + support 3630

TODO items (other than cleanups):
* Add on a dependency b/w vdd1 and vdd2 in a clean manner -> probably
  after we cleanup the resource34xx.c, all folks needing a quick hack
  can easily follow the following pseudo logic:
  resource34xx.c::set_opp:

  if (cpu_is_omap3630()) {
	  vdd1_threshold_freq = 600mhz (opp2)
	  vdd2_required_freq = 200Mhz
  } else {
	  vdd1_threshold_freq = 500 Mhz (opp3)
	  vdd2_required_freq = 100Mhz
  }
  if (vdd1_target < vdd1_threshold_freq)
	free_vdd2_dep_if_acquired();
  /* Do other stuff */
  if (vdd1_target_freq >= vdd1_threshold_freq)
	request_vdd2(at least vdd2_required_freq)

  The 3630 vdd1->vdd2 dependency comes from the OPP table dependency
  of OPP100 and above on VDD1 requires VDD2 to be OPP100. VDD2 is free
  to go as it pleases, but optimal is to be at OPP50 if VDD1 is at
  OPP50. Lots of possible hacks are possible, and I dont recommend any
  at the moment.
* OMAP3630: Add speedbin detection and enable support for OPP-Turbo and
  OPP-SB

Finally,
NOTE: SDP3630 needs the patch for 8250 as discussed here:
http://marc.info/?l=linux-omap&m=125873296522723&w=2
(pushed to l-o, will be available once pm is rebased)

 ====
This patch series is based on previous discussions:
http://www.mail-archive.com/linux-omap@vger.kernel.org/msg17632.html

I have modified the initial patch From Sanjeev:
http://patchwork.kernel.org/patch/50998/

Other than these, we cannot use the old VDDx_MAX based usage
anymore as 3630 OPPs are now different and runtime handling
is a must-have, hence introducing these accessor functions is not
avoidable.

The changes are incremental and tries to avoid intrusive change
as much as possible.

The OPP accessor functions introduced in this series is hopefully
a start for us to optimize this heavily used path.

An alternate approach for detecting 3630 OPP is using FEATURES
instead of detecting the cpu_type as I have implemented in this series.

Testing: mostly on SDP3430 and SDP3630: using cpufreq-utils, sysfs
vddx_lock, RET/OFF using shell scripts. Further testing requested.
Most of the commands used documented here:
http://elinux.org/OMAP_Power_Management

Reposting the series for consistency as only 3 patches retained the v1
status due to change in accessor function names and nature.

Nishanth Menon (10):
Following are the OPP accessor function introduction:
  omap3: pm: introduce enabled flag to omap_opp
  omap3: pm: introduce opp accessor functions - v4
  omap3: pm: use opp accessor functions for omap34xx - v4

The following are attempts to cleanup existing code from VDDx dep:
  omap3: pm: srf: use opp accessor functions - v4
  omap3: pm: sr: replace get_opp with freq_to_opp - v4
  omap3: pm: use opp accessor functions for omap-target - v4
  omap3: clk: use pm accessor functions for cpufreq table - v4
  omap3: pm: remove VDDx_MIN/MAX macros

The following two introduce 3630 support:
  omap3: pm: introduce 3630 opps - v4
  omap3: pm: omap3630 boards: enable 3630 opp tables

 arch/arm/mach-omap2/board-3430sdp.c        |    1 +
 arch/arm/mach-omap2/board-3630sdp.c        |    9 +-
 arch/arm/mach-omap2/board-omap3beagle.c    |    1 +
 arch/arm/mach-omap2/board-omap3evm.c       |    1 +
 arch/arm/mach-omap2/board-rx51.c           |    1 +
 arch/arm/mach-omap2/board-zoom2.c          |    2 +
 arch/arm/mach-omap2/board-zoom3.c          |    9 +-
 arch/arm/mach-omap2/clock34xx.c            |   40 +++--
 arch/arm/mach-omap2/omap3-opp.h            |   58 +------
 arch/arm/mach-omap2/pm.h                   |    6 +
 arch/arm/mach-omap2/pm34xx.c               |  106 +++++++++++
 arch/arm/mach-omap2/resource34xx.c         |  238 ++++++++++++++++++-------
 arch/arm/mach-omap2/smartreflex.c          |   44 ++---
 arch/arm/plat-omap/Makefile                |    3 +
 arch/arm/plat-omap/cpu-omap.c              |   12 +-
 arch/arm/plat-omap/include/plat/omap-pm.h  |   15 +--
 arch/arm/plat-omap/include/plat/omap34xx.h |    5 -
 arch/arm/plat-omap/include/plat/opp.h      |  230 +++++++++++++++++++++++
 arch/arm/plat-omap/opp.c                   |  271 ++++++++++++++++++++++++++++
 19 files changed, 863 insertions(+), 189 deletions(-)
 create mode 100644 arch/arm/plat-omap/include/plat/opp.h
 create mode 100644 arch/arm/plat-omap/opp.c

Regards,
Nishanth Menon

[PATCH 01/10] omap3: pm: introduce enabled flag to omap_opp

[Nishanth Menon]

We used to enable and disable OPPs based on rate being set to 0, this
has been confusing in general. So, we now allow specific OPPs to be
now enabled/disabled by an explicit enabled flag instead of re-using
rate flag itself.

Cc: Benoit Cousson <b-cousson@ti.com>
Cc: Eduardo Valentin <eduardo.valentin@nokia.com>
Cc: Kevin Hilman <khilman@deeprootsystems.com>
Cc: Madhusudhan Chikkature Rajashekar <madhu.cr@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Romit Dasgupta <romit@ti.com>
Cc: Sanjeev Premi <premi@ti.com>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Sergio Alberto Aguirre Rodriguez <saaguirre@ti.com>
Cc: Tero Kristo <Tero.Kristo@nokia.com>
Cc: Thara Gopinath <thara@ti.com>
Cc: Vishwanath Sripathy <vishwanath.bs@ti.com>

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 arch/arm/mach-omap2/omap3-opp.h           |   32 ++++++++++++++--------------
 arch/arm/mach-omap2/resource34xx.c        |    4 +++
 arch/arm/plat-omap/include/plat/omap-pm.h |    2 +
 3 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/arch/arm/mach-omap2/omap3-opp.h b/arch/arm/mach-omap2/omap3-opp.h
index c773ea7..42557e1 100644
--- a/arch/arm/mach-omap2/omap3-opp.h
+++ b/arch/arm/mach-omap2/omap3-opp.h
@@ -22,41 +22,41 @@
 #define S166M   166000000
 
 static struct omap_opp omap3_mpu_rate_table[] = {
-	{0, 0, 0},
+	{0, 0, 0, 0},
 	/*OPP1*/
-	{S125M, VDD1_OPP1, 0x1E},
+	{true, S125M, VDD1_OPP1, 0x1E},
 	/*OPP2*/
-	{S250M, VDD1_OPP2, 0x26},
+	{true, S250M, VDD1_OPP2, 0x26},
 	/*OPP3*/
-	{S500M, VDD1_OPP3, 0x30},
+	{true, S500M, VDD1_OPP3, 0x30},
 	/*OPP4*/
-	{S550M, VDD1_OPP4, 0x36},
+	{true, S550M, VDD1_OPP4, 0x36},
 	/*OPP5*/
-	{S600M, VDD1_OPP5, 0x3C},
+	{true, S600M, VDD1_OPP5, 0x3C},
 };
 
 static struct omap_opp omap3_l3_rate_table[] = {
-	{0, 0, 0},
+	{0, 0, 0, 0},
 	/*OPP1*/
-	{0, VDD2_OPP1, 0x1E},
+	{false, 0, VDD2_OPP1, 0x1E},
 	/*OPP2*/
-	{S83M, VDD2_OPP2, 0x24},
+	{true, S83M, VDD2_OPP2, 0x24},
 	/*OPP3*/
-	{S166M, VDD2_OPP3, 0x2C},
+	{true, S166M, VDD2_OPP3, 0x2C},
 };
 
 static struct omap_opp omap3_dsp_rate_table[] = {
-	{0, 0, 0},
+	{0, 0, 0, 0},
 	/*OPP1*/
-	{S90M, VDD1_OPP1, 0x1E},
+	{true, S90M, VDD1_OPP1, 0x1E},
 	/*OPP2*/
-	{S180M, VDD1_OPP2, 0x26},
+	{true, S180M, VDD1_OPP2, 0x26},
 	/*OPP3*/
-	{S360M, VDD1_OPP3, 0x30},
+	{true, S360M, VDD1_OPP3, 0x30},
 	/*OPP4*/
-	{S400M, VDD1_OPP4, 0x36},
+	{true, S400M, VDD1_OPP4, 0x36},
 	/*OPP5*/
-	{S430M, VDD1_OPP5, 0x3C},
+	{true, S430M, VDD1_OPP5, 0x3C},
 };
 
 #endif
diff --git a/arch/arm/mach-omap2/resource34xx.c b/arch/arm/mach-omap2/resource34xx.c
index 04be4d2..af6b3c1 100644
--- a/arch/arm/mach-omap2/resource34xx.c
+++ b/arch/arm/mach-omap2/resource34xx.c
@@ -285,6 +285,10 @@ static int program_opp(int res, struct omap_opp *opp, int target_level,
 	c_opp = ID_VDD(res) | ID_OPP_NO(opp[current_level].opp_id);
 #endif
 
+	/* Only allow enabled OPPs */
+	if (!opp[target_level].enabled)
+		return -EINVAL;
+
 	/* Sanity check of the OPP params before attempting to set */
 	if (!opp[target_level].rate || !opp[target_level].vsel)
 		return -EINVAL;
diff --git a/arch/arm/plat-omap/include/plat/omap-pm.h b/arch/arm/plat-omap/include/plat/omap-pm.h
index 583e540..5dc2048 100644
--- a/arch/arm/plat-omap/include/plat/omap-pm.h
+++ b/arch/arm/plat-omap/include/plat/omap-pm.h
@@ -21,6 +21,7 @@
 
 /**
  * struct omap_opp - clock frequency-to-OPP ID table for DSP, MPU
+ * @enabled: enabled if true, disabled if false
  * @rate: target clock rate
  * @opp_id: OPP ID
  * @min_vdd: minimum VDD1 voltage (in millivolts) for this OPP
@@ -28,6 +29,7 @@
  * Operating performance point data.  Can vary by OMAP chip and board.
  */
 struct omap_opp {
+	bool enabled;
 	unsigned long rate;
 	u8 opp_id;
 	u16 vsel;
-- 
1.6.3.3

[PATCH 02/10 V4] omap3: pm: introduce opp accessor functions

[Nishanth Menon]

Modifies the initial patch From Sanjeev:
http://patchwork.kernel.org/patch/50998/
Discussions and comments from:
http://marc.info/?l=linux-omap&m=125482970102327&w=2
http://marc.info/?t=125809247500002&r=1&w=2
http://marc.info/?l=linux-omap&m=126025973426007&w=2
incorporated.

OMAP SOCs have a standard set of tuples consisting of frequency and
voltage pairs that the device will support per voltage domain. This
is called Operating Points or OPP. The actual definitions of OMAP
Operating Points varies over silicon within the same family of
devices. For a specific domain, you can have a set of
{frequency, voltage} pairs. As the kernel boots and more information
is available, a set of these are activated based on the precise
nature of device the kernel boots up on. It is interesting to
remember that each IP which belongs to a voltage domain may define
their own set of OPPs on top of this.

This introduces a common handling OPP mechanism accross all OMAPs.
As a start this is introduced for OMAP3 and intends to replace
current OMAP3 opp handling mechanism.

Note:
fields of struct omap_opp is currently exposed due to the necessity
that SRF and SR logic directly indexes the structure array fields.
The goal however, is to make the direct usage of omap_opp deprecated
and move to using these accessor functions. The usage in SRF and SR
indexes based on opp_id and hence opp_id is marked deprecated to
generate build warnings at least. Further, this usage necessitates
need of terminator entries at the start and end of opp_* tables which
are dynamically allocated.

The accessor function definitions were collaborated with Kevin, and
doing justice here, this implementation could not go with some of
the better suggestions from kevin due to constraint imposed by SRF
and SR. A better and more optimal implementation is definitely
possible once SRF and SR are cleanedup/replaced.

NOTE: OPP is a concept that can be used in all OMAPs, it is hence
introduced under plat-omap

Introduces warning:
arch/arm/plat-omap/opp.c: In function 'opp_add':
arch/arm/plat-omap/opp.c:191: warning: 'opp_id' is deprecated (declared at arch/arm/plat-omap/include/plat/opp.h:33)
arch/arm/plat-omap/opp.c:199: warning: 'opp_id' is deprecated (declared at arch/arm/plat-omap/include/plat/opp.h:33)
arch/arm/plat-omap/opp.c: In function 'opp_init_list':
arch/arm/plat-omap/opp.c:240: warning: 'opp_id' is deprecated (declared at arch/arm/plat-omap/include/plat/opp.h:33)

Cc: Benoit Cousson <b-cousson@ti.com>
Cc: Eduardo Valentin <eduardo.valentin@nokia.com>
Cc: Madhusudhan Chikkature Rajashekar <madhu.cr@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Romit Dasgupta <romit@ti.com>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Sergio Alberto Aguirre Rodriguez <saaguirre@ti.com>
Cc: Tero Kristo <Tero.Kristo@nokia.com>
Cc: Thara Gopinath <thara@ti.com>
Cc: Vishwanath Sripathy <vishwanath.bs@ti.com>

Signed-off-by: Sanjeev Premi <premi@ti.com>
Signed-off-by: Kevin Hilman <khilman@deeprootsystems.com>
Signed-off-by: Nishanth Menon <nm@ti.com>
---
Kevin,
I have put your Signed-off-by to ack your contribs. pending
formal confirmation ofcourse.

 arch/arm/plat-omap/Makefile           |    3 +
 arch/arm/plat-omap/include/plat/opp.h |  230 ++++++++++++++++++++++++++++
 arch/arm/plat-omap/opp.c              |  271 +++++++++++++++++++++++++++++++++
 3 files changed, 504 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/plat-omap/include/plat/opp.h
 create mode 100644 arch/arm/plat-omap/opp.c

diff --git a/arch/arm/plat-omap/Makefile b/arch/arm/plat-omap/Makefile
index 95f8413..e9cf601 100644
--- a/arch/arm/plat-omap/Makefile
+++ b/arch/arm/plat-omap/Makefile
@@ -12,6 +12,9 @@ obj-  :=
 # OCPI interconnect support for 1710, 1610 and 5912
 obj-$(CONFIG_ARCH_OMAP16XX) += ocpi.o
 
+# OPP support in (OMAP3+ only at the moment)
+obj-$(CONFIG_ARCH_OMAP3) += opp.o
+
 # omap_device support (OMAP2+ only at the moment)
 obj-$(CONFIG_ARCH_OMAP2) += omap_device.o
 obj-$(CONFIG_ARCH_OMAP3) += omap_device.o
diff --git a/arch/arm/plat-omap/include/plat/opp.h b/arch/arm/plat-omap/include/plat/opp.h
new file mode 100644
index 0000000..341c02b
--- /dev/null
+++ b/arch/arm/plat-omap/include/plat/opp.h
@@ -0,0 +1,230 @@
+/*
+ * OMAP OPP Interface
+ *
+ * Copyright (C) 2009 Texas Instruments Incorporated.
+ *	Nishanth Menon
+ * Copyright (C) 2009 Deep Root Systems, LLC.
+ *	Kevin Hilman
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#ifndef __ASM_ARM_OMAP_OPP_H
+#define __ASM_ARM_OMAP_OPP_H
+
+/**
+ * struct omap_opp - OMAP OPP description structure
+ * @enabled:	true/false - marking this OPP as enabled/disabled
+ * @rate:	Frequency in hertz
+ * @opp_id:	(DEPRECATED) opp identifier
+ * @vsel:	Voltage in volt processor level(this usage is
+ *		DEPRECATED to use Voltage in microvolts in future)
+ *		uV = ((vsel * 12.5) + 600) * 1000
+ *
+ * This structure stores the OPP information for a given domain.
+ * Due to legacy reasons, this structure is currently exposed and
+ * will soon be removed elsewhere and will only be used as a handle
+ * from the OPP internal referencing mechanism
+ */
+struct omap_opp {
+	bool enabled;
+	unsigned long rate;
+	u8 opp_id __deprecated;
+	u16 vsel;
+};
+
+/**
+ * opp_get_voltage() - Gets the voltage corresponding to an opp
+ * @opp:	opp for which voltage has to be returned for
+ *
+ * Return voltage in micro volt corresponding to the opp, else
+ * return 0
+ */
+unsigned long opp_get_voltage(const struct omap_opp *opp);
+
+/**
+ * opp_get_freq() - Gets the frequency corresponding to an opp
+ * @opp:	opp for which frequency has to be returned for
+ *
+ * Return frequency in hertz corresponding to the opp, else
+ * return 0
+ */
+unsigned long opp_get_freq(const struct omap_opp *opp);
+
+/**
+ * opp_get_opp_count() - Get number of opps enabled in the opp list
+ * @num:	returns the number of opps
+ * @oppl:	opp list
+ *
+ * This functions returns the number of opps if there are any OPPs enabled,
+ * else returns corresponding error value.
+ */
+int opp_get_opp_count(const struct omap_opp *oppl);
+
+/**
+ * opp_find_freq_exact() - search for an exact frequency
+ * @oppl:	OPP list
+ * @freq:	frequency to search for
+ * @enabled:	enabled/disabled OPP to search for
+ *
+ * searches for the match in the opp list and returns handle to the matching
+ * opp if found, else returns ERR_PTR in case of error and should be handled
+ * using IS_ERR.
+ *
+ * Note enabled is a modifier for the search. if enabled=true, then the match is
+ * for exact matching frequency and is enabled. if true, the match is for exact
+ * frequency which is disabled.
+ */
+struct omap_opp *opp_find_freq_exact(struct omap_opp *oppl,
+				     unsigned long freq, bool enabled);
+
+#define OPP_SEARCH_HIGH		(0 << 1)
+#define OPP_SEARCH_LOW		(1 << 1)
+/**
+ * opp_find_freq_approx() - Search for an rounded freq
+ * @oppl:	Starting list
+ * @freq:	Start frequency
+ * @dir_flag:	Search direction
+ *		OPP_SEARCH_HIGH - search for next highest freq
+ *		OPP_SEARCH_LOW - search for next lowest freq
+ *
+ * Search for the higher/lower *enabled* OPP from a starting freq
+ * from a start opp list.
+ *
+ * Returns *opp and *freq is populated with the next match,
+ * else returns NULL
+ * opp if found, else returns ERR_PTR in case of error.
+ *
+ * Example usages:
+ *	* find match/next highest available frequency
+ *	freq = 350000;
+ *	opp = opp_find_freq_approx(oppl, &freq, OPP_SEARCH_HIGH)))
+ *	if (IS_ERR(opp))
+ *		pr_err ("unable to find a higher frequency\n");
+ *	else
+ *		pr_info("match freq = %ld\n", freq);
+ *
+ *	* find match/next lowest available frequency
+ *	freq = 350000;
+ *	opp = opp_find_freq_approx(oppl, &freq, OPP_SEARCH_LOW)))
+ *	if (IS_ERR(opp))
+ *		pr_err ("unable to find a lower frequency\n");
+ *	else
+ *		pr_info("match freq = %ld\n", freq);
+ *
+ *	* print all supported frequencies in descending order *
+ *	opp = oppl;
+ *	freq = ULONG_MAX;
+ *	while (!IS_ERR(opp = opp_find_freq_approx(opp, &freq,
+ *		OPP_SEARCH_LOW))) {
+ *		pr_info("freq = %ld\n", freq);
+ *		freq--; * for next lower match *
+ *	}
+ *
+ *	* print all supported frequencies in ascending order *
+ *	opp = oppl;
+ *	freq = 0;
+ *	while (!IS_ERR(opp = opp_find_freq_approx(opp, &freq,
+ *			OPP_SEARCH_HIGH))) {
+ *		pr_info("freq = %ld\n", freq);
+ *		freq++; * for next higher match *
+ *	}
+ *
+ * NOTE: if we set freq as ULONG_MAX and search low, we get the highest enabled
+ * frequency
+ */
+struct omap_opp *opp_find_freq_approx(struct omap_opp *oppl,
+				      unsigned long *freq, u8 dir_flag);
+
+/**
+ * struct omap_opp_def - OMAP OPP Definition
+ * @enabled:	True/false - is this OPP enabled/disabled by default
+ * @freq:	Frequency in hertz corresponding to this OPP
+ * @u_volt:	Nominal voltage in microvolts corresponding to this OPP
+ *
+ * OMAP SOCs have a standard set of tuples consisting of frequency and voltage
+ * pairs that the device will support per voltage domain. This is called
+ * Operating Points or OPP. The actual definitions of OMAP Operating Points
+ * varies over silicon within the same family of devices. For a specific
+ * domain, you can have a set of {frequency, voltage} pairs and this is denoted
+ * by an array of omap_opp_def. As the kernel boots and more information is
+ * available, a set of these are activated based on the precise nature of
+ * device the kernel boots up on. It is interesting to remember that each IP
+ * which belongs to a voltage domain may define their own set of OPPs on top
+ * of this - but this is handled by the appropriate driver.
+ */
+struct omap_opp_def {
+	bool enabled;
+	unsigned long freq;
+	u32 u_volt;
+};
+
+/* Initialization wrapper */
+#define OMAP_OPP_DEF(_enabled, _freq, _uv)	\
+{						\
+	.enabled	= _enabled,		\
+	.freq		= _freq,		\
+	.u_volt		= _uv,			\
+}
+
+/* Terminator for the initialization list */
+#define OMAP_OPP_DEF_TERMINATOR OMAP_OPP_DEF(0, 0, 0)
+
+/**
+ * opp_init_list() - Initialize an opp list from the opp definitions
+ * @opp_defs:	Initial opp definitions to create the list.
+ *
+ * This function creates a list of opp definitions and returns a handle.
+ * This list can be used to further validation/search/modifications. New
+ * opp entries can be added to this list by using opp_add().
+ *
+ * In the case of error, ERR_PTR is returned to the caller and should be
+ * appropriately handled with IS_ERR.
+ */
+struct omap_opp __init *opp_init_list(const struct omap_opp_def *opp_defs);
+
+/**
+ * opp_add()  - Add an OPP table from a table definitions
+ * @oppl:	List to add the OPP to
+ * @opp_def:	omap_opp_def to describe the OPP which we want to add to list.
+ *
+ * This function adds an opp definition to the opp list and returns
+ * a handle representing the new OPP list. This handle is then used for further
+ * validation, search, modification operations on the OPP list.
+ *
+ * This function returns the pointer to the allocated list through oppl if
+ * success, else corresponding ERR_PTR value. Caller should NOT free the oppl.
+ * opps_defs can be freed after use.
+ *
+ * NOTE: caller should assume that on success, oppl is probably populated with
+ * a new handle and the new handle should be used for further referencing
+ */
+struct omap_opp *opp_add(struct omap_opp *oppl,
+			 const struct omap_opp_def *opp_def);
+
+/**
+ * opp_enable() - Enable a specific OPP
+ * @opp:	Pointer to opp
+ *
+ * Enables a provided opp. If the operation is valid, this returns 0, else the
+ * corresponding error value.
+ *
+ * OPP used here is from the the opp_is_valid/opp_has_freq or other search
+ * functions
+ */
+int opp_enable(struct omap_opp *opp);
+
+/**
+ * opp_disable() - Disable a specific OPP
+ * @opp:	Pointer to opp
+ *
+ * Disables a provided opp. If the operation is valid, this returns 0, else the
+ * corresponding error value.
+ *
+ * OPP used here is from the the opp_is_valid/opp_has_freq or other search
+ * functions
+ */
+int opp_disable(struct omap_opp *opp);
+
+#endif		/* __ASM_ARM_OMAP_OPP_H */
diff --git a/arch/arm/plat-omap/opp.c b/arch/arm/plat-omap/opp.c
new file mode 100644
index 0000000..c4dc07b
--- /dev/null
+++ b/arch/arm/plat-omap/opp.c
@@ -0,0 +1,271 @@
+/*
+ * OMAP OPP Interface
+ *
+ * Copyright (C) 2009 Texas Instruments Incorporated.
+ *	Nishanth Menon
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+
+#include <plat/opp.h>
+
+/*
+ * DEPRECATED: Meant to detect end of opp array
+ * This is meant to help co-exist with current SRF etc
+ * TODO: REMOVE!
+ */
+#define OPP_TERM(opp) (!(opp)->rate && !(opp)->vsel && !(opp)->enabled)
+
+/*
+ * DEPRECATED: Meant to convert vsel value to uVolt
+ * This is meant to help co-exist with current SRF etc
+ * TODO: REMOVE!
+ */
+static inline unsigned long vsel_to_uv(const u8 vsel)
+{
+	return (((vsel * 125) + 6000)) * 100;
+}
+
+/*
+ * DEPRECATED: Meant to convert uVolt to vsel value
+ * This is meant to help co-exist with current SRF etc
+ * TODO: REMOVE!
+ */
+static inline unsigned char uv_to_vsel(unsigned long uV)
+{
+	return ((uV / 100) - 6000) / 125;
+}
+
+unsigned long opp_get_voltage(const struct omap_opp *opp)
+{
+	if (unlikely(!opp || IS_ERR(opp)) || !opp->enabled) {
+		pr_err("%s: Invalid parameters being passed\n", __func__);
+		return 0;
+	}
+	return vsel_to_uv(opp->vsel);
+}
+
+unsigned long opp_get_freq(const struct omap_opp *opp)
+{
+	if (unlikely(!opp || IS_ERR(opp)) || !opp->enabled) {
+		pr_err("%s: Invalid parameters being passed\n", __func__);
+		return 0;
+	}
+	return opp->rate;
+}
+
+int opp_get_opp_count(const struct omap_opp *oppl)
+{
+	struct omap_opp *opp;
+	u8 n = 0;
+
+	if (unlikely(!oppl || IS_ERR(oppl))) {
+		pr_err("%s: Invalid parameters being passed\n", __func__);
+		return -EINVAL;
+	}
+	opp = (struct omap_opp *)oppl;
+	opp++;			/* skip initial terminator */
+	while (!OPP_TERM(opp)) {
+		if (opp->enabled)
+			n++;
+		opp++;
+	}
+	return n;
+}
+
+struct omap_opp *opp_find_freq_exact(struct omap_opp *oppl,
+				     unsigned long freq, bool enabled)
+{
+	struct omap_opp *opp = (struct omap_opp *)oppl;
+
+	if (unlikely(!oppl || IS_ERR(oppl))) {
+		pr_err("%s: Invalid parameters being passed\n", __func__);
+		return ERR_PTR(-EINVAL);
+	}
+	/* skip initial terminator */
+	if (OPP_TERM(opp))
+		opp++;
+	while (!OPP_TERM(opp)) {
+		if ((opp->rate == freq) && (opp->enabled == enabled))
+			break;
+		opp++;
+	}
+
+	return OPP_TERM(opp) ? ERR_PTR(-ENOENT) : opp;
+}
+
+struct omap_opp *opp_find_freq_approx(struct omap_opp *oppl,
+				      unsigned long *freq, u8 dir_flag)
+{
+	struct omap_opp *opp = (struct omap_opp *)oppl;
+
+	if (unlikely(!oppl || IS_ERR(oppl) || !freq || IS_ERR(freq))) {
+		pr_err("%s: Invalid parameters being passed\n", __func__);
+		return ERR_PTR(-EINVAL);
+	}
+	/* skip initial terminator */
+	if (OPP_TERM(opp)) {
+		opp++;
+		/* If searching init list for a high val, skip to very top */
+		if (dir_flag == OPP_SEARCH_LOW)
+			while (!OPP_TERM(opp + 1))
+				opp++;
+	}
+	while (!OPP_TERM(opp)) {
+		if (opp->enabled &&
+		    (((dir_flag == OPP_SEARCH_HIGH) && (opp->rate >= *freq)) ||
+		     ((dir_flag == OPP_SEARCH_LOW) && (opp->rate <= *freq))))
+			break;
+		opp += (dir_flag == OPP_SEARCH_LOW) ? -1 : 1;
+	}
+
+	if (OPP_TERM(opp))
+		return ERR_PTR(-ENOENT);
+
+	*freq = opp->rate;
+	return opp;
+}
+
+/* wrapper to reuse converting opp_def to opp struct */
+static void omap_opp_populate(struct omap_opp *opp,
+			      const struct omap_opp_def *opp_def)
+{
+	opp->rate = opp_def->freq;
+	opp->enabled = opp_def->enabled;
+	opp->vsel = uv_to_vsel(opp_def->u_volt);
+	/* round off to higher voltage */
+	if (opp_def->u_volt > vsel_to_uv(opp->vsel))
+		opp->vsel++;
+}
+
+struct omap_opp *opp_add(struct omap_opp *oppl,
+			 const struct omap_opp_def *opp_def)
+{
+	struct omap_opp *opp, *oppt, *oppr;
+	u8 n, i, ins;
+
+	if (unlikely(!oppl || IS_ERR(oppl) || !opp_def || IS_ERR(opp_def))) {
+		pr_err("%s: Invalid params being passed\n", __func__);
+		return ERR_PTR(-EINVAL);
+	}
+	/* need a start terminator.. */
+	if (unlikely(!OPP_TERM(oppl))) {
+		pr_err("%s: Expected a start terminator!!\n", __func__);
+		return ERR_PTR(-EINVAL);
+	}
+	n = 0;
+	opp = oppl;
+	opp++;
+	while (!OPP_TERM(opp)) {
+		n++;
+		opp++;
+	}
+	/* lets now reallocate memory */
+	oppr = kmalloc(sizeof(struct omap_opp) * (n + 3), GFP_KERNEL);
+	if (!oppr) {
+		pr_err("%s: No memory for new opp array\n", __func__);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	/* Simple insertion sort */
+	opp = oppl;
+	oppt = oppr;
+	ins = 0;
+	i = 0;
+	do {
+		if (ins || opp->rate < opp_def->freq) {
+			memcpy(oppt, opp, sizeof(struct omap_opp));
+			opp++;
+		} else {
+			omap_opp_populate(oppt, opp_def);
+			ins++;
+		}
+		oppt->opp_id = i;
+		oppt++;
+		i++;
+	} while (!OPP_TERM(opp));
+
+	/* If nothing got inserted, this belongs to the end */
+	if (!ins) {
+		omap_opp_populate(oppt, opp_def);
+		oppt->opp_id = i;
+		oppt++;
+	}
+	/* Put the terminator back on */
+	memcpy(oppt, opp, sizeof(struct omap_opp));
+
+	/* Free the old list */
+	kfree(oppl);
+
+	return oppr;
+}
+
+struct omap_opp __init *opp_init_list(const struct omap_opp_def *opp_defs)
+{
+	struct omap_opp_def *t = (struct omap_opp_def *)opp_defs;
+	struct omap_opp *opp, *oppl;
+	u8 n = 0, i = 1;
+
+	if (unlikely(!opp_defs || IS_ERR(opp_defs))) {
+		pr_err("%s: Invalid params being passed\n", __func__);
+		return ERR_PTR(-EINVAL);
+	}
+	/* Grab a count */
+	while (t->enabled || t->freq || t->u_volt) {
+		n++;
+		t++;
+	}
+
+	oppl = kmalloc(sizeof(struct omap_opp) * (n + 2), GFP_KERNEL);
+	if (!oppl) {
+		pr_err("%s: No memory for opp array\n", __func__);
+		return ERR_PTR(-ENOMEM);
+	}
+	opp = oppl;
+	/* Setup start terminator - SRF depends on this for indexing :( */
+	opp->rate = 0;
+	opp->enabled = 0;
+	opp->vsel = 0;
+	opp++;
+	while (n) {
+		omap_opp_populate(opp, opp_defs);
+		opp->opp_id = i;
+		n--;
+		opp++;
+		opp_defs++;
+		i++;
+	}
+	/* Setup terminator - this is for our search algos */
+	opp->rate = 0;
+	opp->enabled = 0;
+	opp->vsel = 0;
+	return oppl;
+}
+
+int opp_enable(struct omap_opp *opp)
+{
+	if (unlikely(!opp || IS_ERR(opp))) {
+		pr_err("%s: Invalid parameters being passed\n", __func__);
+		return -EINVAL;
+	}
+	opp->enabled = true;
+	return 0;
+}
+
+int opp_disable(struct omap_opp *opp)
+{
+	if (unlikely(!opp || IS_ERR(opp))) {
+		pr_err("%s: Invalid parameters being passed\n", __func__);
+		return -EINVAL;
+	}
+	opp->enabled = false;
+	return 0;
+}
-- 
1.6.3.3

[PATCH 03/10 V4] omap3: pm: use opp accessor functions for omap34xx

[Nishanth Menon]

Move the definitions from omap3-opp.h to pm34xx.c. The definitions
are now based on omap_opp_def instead of omap_opp itself.
Since the opp.h has the omap_opp definition, omap-pm.h conflicts and
has been removed in favor of opp.h.

omap3_pm_init_opp_table is used to initialize the OPP table and
relevant board files which have omap2_init_common_hw called with opp
arrays have been updated with omap3_pm_init_opp_table.

This change now allows us to dynamically register OPPs to the system
based on silicon type we detect.

NOTE: This introduces the following warnings highlighting areas we
need to cleanup:
arch/arm/mach-omap2/smartreflex.c: In function 'get_opp':
arch/arm/mach-omap2/smartreflex.c:161: warning: 'opp_id' is deprecated (declared at arch/arm/plat-omap/include/plat/opp.h:33)
arch/arm/mach-omap2/smartreflex.c:164: warning: 'opp_id' is deprecated (declared at arch/arm/plat-omap/include/plat/opp.h:33)
arch/arm/mach-omap2/smartreflex.c:166: warning: 'opp_id' is deprecated (declared at arch/arm/plat-omap/include/plat/opp.h:33)
arch/arm/mach-omap2/smartreflex.c:168: warning: 'opp_id' is deprecated (declared at arch/arm/plat-omap/include/plat/opp.h:33)
arch/arm/mach-omap2/resource34xx.c: In function 'get_opp':
arch/arm/mach-omap2/resource34xx.c:165: warning: 'opp_id' is deprecated (declared at arch/arm/plat-omap/include/plat/opp.h:33)
arch/arm/mach-omap2/resource34xx.c:168: warning: 'opp_id' is deprecated (declared at arch/arm/plat-omap/include/plat/opp.h:33)
arch/arm/mach-omap2/resource34xx.c:170: warning: 'opp_id' is deprecated (declared at arch/arm/plat-omap/include/plat/opp.h:33)
arch/arm/mach-omap2/resource34xx.c:172: warning: 'opp_id' is deprecated (declared at arch/arm/plat-omap/include/plat/opp.h:33)
arch/arm/mach-omap2/resource34xx.c: In function 'program_opp':
arch/arm/mach-omap2/resource34xx.c:284: warning: 'opp_id' is deprecated (declared at arch/arm/plat-omap/include/plat/opp.h:33)
arch/arm/mach-omap2/resource34xx.c:285: warning: 'opp_id' is deprecated (declared at arch/arm/plat-omap/include/plat/opp.h:33)

Cc: Benoit Cousson <b-cousson@ti.com>
Cc: Eduardo Valentin <eduardo.valentin@nokia.com>
Cc: Kevin Hilman <khilman@deeprootsystems.com>
Cc: Madhusudhan Chikkature Rajashekar <madhu.cr@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Romit Dasgupta <romit@ti.com>
Cc: Sanjeev Premi <premi@ti.com>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Sergio Alberto Aguirre Rodriguez <saaguirre@ti.com>
Cc: Tero Kristo <Tero.Kristo@nokia.com>
Cc: Thara Gopinath <thara@ti.com>
Cc: Vishwanath Sripathy <vishwanath.bs@ti.com>

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 arch/arm/mach-omap2/board-3430sdp.c       |    1 +
 arch/arm/mach-omap2/board-omap3beagle.c   |    1 +
 arch/arm/mach-omap2/board-omap3evm.c      |    1 +
 arch/arm/mach-omap2/board-rx51.c          |    1 +
 arch/arm/mach-omap2/board-zoom2.c         |    2 +
 arch/arm/mach-omap2/omap3-opp.h           |   58 +------------------------
 arch/arm/mach-omap2/pm.h                  |    6 +++
 arch/arm/mach-omap2/pm34xx.c              |   65 +++++++++++++++++++++++++++++
 arch/arm/plat-omap/include/plat/omap-pm.h |   17 +-------
 9 files changed, 81 insertions(+), 71 deletions(-)

diff --git a/arch/arm/mach-omap2/board-3430sdp.c b/arch/arm/mach-omap2/board-3430sdp.c
index eac529f..0ec8327 100644
--- a/arch/arm/mach-omap2/board-3430sdp.c
+++ b/arch/arm/mach-omap2/board-3430sdp.c
@@ -220,6 +220,7 @@ static void __init omap_3430sdp_init_irq(void)
 {
 	omap_board_config = sdp3430_config;
 	omap_board_config_size = ARRAY_SIZE(sdp3430_config);
+	omap3_pm_init_opp_table();
 	omap3_pm_init_vc(&omap3_setuptime_table);
 	omap3_pm_init_cpuidle(omap3_cpuidle_params_table);
 	omap2_init_common_hw(hyb18m512160af6_sdrc_params, NULL, omap3_mpu_rate_table,
diff --git a/arch/arm/mach-omap2/board-omap3beagle.c b/arch/arm/mach-omap2/board-omap3beagle.c
index 2ec3520..a937238 100644
--- a/arch/arm/mach-omap2/board-omap3beagle.c
+++ b/arch/arm/mach-omap2/board-omap3beagle.c
@@ -361,6 +361,7 @@ static void __init omap3_beagle_init_irq(void)
 {
 	omap_board_config = omap3_beagle_config;
 	omap_board_config_size = ARRAY_SIZE(omap3_beagle_config);
+	omap3_pm_init_opp_table();
 	omap2_init_common_hw(mt46h32m32lf6_sdrc_params,
 			     mt46h32m32lf6_sdrc_params, omap3_mpu_rate_table,
 			     omap3_dsp_rate_table, omap3_l3_rate_table);
diff --git a/arch/arm/mach-omap2/board-omap3evm.c b/arch/arm/mach-omap2/board-omap3evm.c
index 8130eca..44a5861 100644
--- a/arch/arm/mach-omap2/board-omap3evm.c
+++ b/arch/arm/mach-omap2/board-omap3evm.c
@@ -404,6 +404,7 @@ static void __init omap3_evm_init_irq(void)
 {
 	omap_board_config = omap3_evm_config;
 	omap_board_config_size = ARRAY_SIZE(omap3_evm_config);
+	omap3_pm_init_opp_table();
 	omap2_init_common_hw(mt46h32m32lf6_sdrc_params, NULL, omap3_mpu_rate_table,
 	                     omap3_dsp_rate_table, omap3_l3_rate_table);
 	omap_init_irq();
diff --git a/arch/arm/mach-omap2/board-rx51.c b/arch/arm/mach-omap2/board-rx51.c
index 2f1c2be..997fd1c 100644
--- a/arch/arm/mach-omap2/board-rx51.c
+++ b/arch/arm/mach-omap2/board-rx51.c
@@ -103,6 +103,7 @@ static void __init rx51_init_irq(void)
 
 	omap_board_config = rx51_config;
 	omap_board_config_size = ARRAY_SIZE(rx51_config);
+	omap3_pm_init_opp_table();
 	omap3_pm_init_cpuidle(rx51_cpuidle_params);
 	sdrc_params = rx51_get_sdram_timings();
 	omap2_init_common_hw(sdrc_params, sdrc_params,
diff --git a/arch/arm/mach-omap2/board-zoom2.c b/arch/arm/mach-omap2/board-zoom2.c
index dcc5fb8..9d5b078 100644
--- a/arch/arm/mach-omap2/board-zoom2.c
+++ b/arch/arm/mach-omap2/board-zoom2.c
@@ -24,10 +24,12 @@
 #include <mach/board-zoom.h>
 
 #include "sdram-micron-mt46h32m32lf-6.h"
+#include "pm.h"
 #include "omap3-opp.h"
 
 static void __init omap_zoom2_init_irq(void)
 {
+	omap3_pm_init_opp_table();
 	omap2_init_common_hw(mt46h32m32lf6_sdrc_params,
 				 mt46h32m32lf6_sdrc_params, omap3_mpu_rate_table,
 				 omap3_dsp_rate_table, omap3_l3_rate_table);
diff --git a/arch/arm/mach-omap2/omap3-opp.h b/arch/arm/mach-omap2/omap3-opp.h
index 42557e1..994d8d4 100644
--- a/arch/arm/mach-omap2/omap3-opp.h
+++ b/arch/arm/mach-omap2/omap3-opp.h
@@ -3,60 +3,8 @@
 
 #include <plat/omap-pm.h>
 
-/* MPU speeds */
-#define S600M   600000000
-#define S550M   550000000
-#define S500M   500000000
-#define S250M   250000000
-#define S125M   125000000
-
-/* DSP speeds */
-#define S430M   430000000
-#define S400M   400000000
-#define S360M   360000000
-#define S180M   180000000
-#define S90M    90000000
-
-/* L3 speeds */
-#define S83M    83000000
-#define S166M   166000000
-
-static struct omap_opp omap3_mpu_rate_table[] = {
-	{0, 0, 0, 0},
-	/*OPP1*/
-	{true, S125M, VDD1_OPP1, 0x1E},
-	/*OPP2*/
-	{true, S250M, VDD1_OPP2, 0x26},
-	/*OPP3*/
-	{true, S500M, VDD1_OPP3, 0x30},
-	/*OPP4*/
-	{true, S550M, VDD1_OPP4, 0x36},
-	/*OPP5*/
-	{true, S600M, VDD1_OPP5, 0x3C},
-};
-
-static struct omap_opp omap3_l3_rate_table[] = {
-	{0, 0, 0, 0},
-	/*OPP1*/
-	{false, 0, VDD2_OPP1, 0x1E},
-	/*OPP2*/
-	{true, S83M, VDD2_OPP2, 0x24},
-	/*OPP3*/
-	{true, S166M, VDD2_OPP3, 0x2C},
-};
-
-static struct omap_opp omap3_dsp_rate_table[] = {
-	{0, 0, 0, 0},
-	/*OPP1*/
-	{true, S90M, VDD1_OPP1, 0x1E},
-	/*OPP2*/
-	{true, S180M, VDD1_OPP2, 0x26},
-	/*OPP3*/
-	{true, S360M, VDD1_OPP3, 0x30},
-	/*OPP4*/
-	{true, S400M, VDD1_OPP4, 0x36},
-	/*OPP5*/
-	{true, S430M, VDD1_OPP5, 0x3C},
-};
+extern struct omap_opp *omap3_mpu_rate_table;
+extern struct omap_opp *omap3_dsp_rate_table;
+extern struct omap_opp *omap3_l3_rate_table;
 
 #endif
diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
index 7bc86b6..80a1c1d 100644
--- a/arch/arm/mach-omap2/pm.h
+++ b/arch/arm/mach-omap2/pm.h
@@ -58,6 +58,12 @@ static inline void omap3_pm_init_cpuidle(
 {
 }
 #endif
+/**
+ * omap3_pm_init_opp_table - OMAP opp table lookup called after cpu is detected.
+ * Initialize the basic opp table here, board files could choose to modify opp
+ * table after the basic initialization
+ */
+extern void omap3_pm_init_opp_table(void);
 
 extern int resource_set_opp_level(int res, u32 target_level, int flags);
 extern int resource_access_opp_lock(int res, int delta);
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 627a509..e40a036 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -40,6 +40,7 @@
 #include <plat/dmtimer.h>
 #include <plat/usb.h>
 
+#include <plat/opp.h>
 #include <plat/resource.h>
 
 #include <asm/tlbflush.h>
@@ -52,6 +53,7 @@
 #include "prm.h"
 #include "pm.h"
 #include "sdrc.h"
+#include "omap3-opp.h"
 
 static int regset_save_on_suspend;
 
@@ -100,6 +102,49 @@ static struct prm_setup_vc prm_setup = {
 	.vdd1_off = 0x00,	/* 0.6v */
 };
 
+static struct omap_opp_def __initdata omap34xx_mpu_rate_table[] = {
+	/* OPP1 */
+	OMAP_OPP_DEF(true, 125000000, 975000),
+	/* OPP2 */
+	OMAP_OPP_DEF(true, 250000000, 1075000),
+	/* OPP3 */
+	OMAP_OPP_DEF(true, 500000000, 1200000),
+	/* OPP4 */
+	OMAP_OPP_DEF(true, 550000000, 1270000),
+	/* OPP5 */
+	OMAP_OPP_DEF(true, 600000000, 1350000),
+	OMAP_OPP_DEF_TERMINATOR
+};
+
+static struct omap_opp_def __initdata omap34xx_l3_rate_table[] = {
+	/* OPP1 */
+	OMAP_OPP_DEF(false, 0, 975000),
+	/* OPP2 */
+	OMAP_OPP_DEF(true, 83000000, 1050000),
+	/* OPP3 */
+	OMAP_OPP_DEF(true, 166000000, 1150000),
+	OMAP_OPP_DEF_TERMINATOR
+};
+
+static struct omap_opp_def __initdata omap34xx_dsp_rate_table[] = {
+	/* OPP1 */
+	OMAP_OPP_DEF(true, 90000000, 975000),
+	/* OPP2 */
+	OMAP_OPP_DEF(true, 180000000, 1075000),
+	/* OPP3 */
+	OMAP_OPP_DEF(true, 360000000, 1200000),
+	/* OPP4 */
+	OMAP_OPP_DEF(true, 400000000, 1270000),
+	/* OPP5 */
+	OMAP_OPP_DEF(true, 430000000, 1350000),
+	OMAP_OPP_DEF_TERMINATOR
+};
+
+/* OMAP3 Rate Table */
+struct omap_opp *omap3_mpu_rate_table;
+struct omap_opp *omap3_dsp_rate_table;
+struct omap_opp *omap3_l3_rate_table;
+
 static inline void omap3_per_save_context(void)
 {
 	omap_gpio_save_context();
@@ -1248,6 +1293,26 @@ static void __init configure_vc(void)
 	pm_dbg_regset_init(2);
 }
 
+void __init omap3_pm_init_opp_table(void)
+{
+	int i;
+	struct omap_opp_def *omap34xx_opp_def_list[] = {
+		omap34xx_mpu_rate_table,
+		omap34xx_l3_rate_table,
+		omap34xx_dsp_rate_table
+	};
+	struct omap_opp **omap3_rate_tables[] = {
+		&omap3_mpu_rate_table,
+		&omap3_l3_rate_table,
+		&omap3_dsp_rate_table
+	};
+	for (i = 0; i < ARRAY_SIZE(omap3_rate_tables); i++) {
+		*omap3_rate_tables[i] = opp_init_list(omap34xx_opp_def_list[i]);
+		/* We dont want half configured system at the moment */
+		BUG_ON(IS_ERR(omap3_rate_tables[i]));
+	}
+}
+
 static int __init omap3_pm_early_init(void)
 {
 	prm_clear_mod_reg_bits(OMAP3430_OFFMODE_POL, OMAP3430_GR_MOD,
diff --git a/arch/arm/plat-omap/include/plat/omap-pm.h b/arch/arm/plat-omap/include/plat/omap-pm.h
index 5dc2048..aa36339 100644
--- a/arch/arm/plat-omap/include/plat/omap-pm.h
+++ b/arch/arm/plat-omap/include/plat/omap-pm.h
@@ -18,22 +18,7 @@
 #include <linux/cpufreq.h>
 
 #include "powerdomain.h"
-
-/**
- * struct omap_opp - clock frequency-to-OPP ID table for DSP, MPU
- * @enabled: enabled if true, disabled if false
- * @rate: target clock rate
- * @opp_id: OPP ID
- * @min_vdd: minimum VDD1 voltage (in millivolts) for this OPP
- *
- * Operating performance point data.  Can vary by OMAP chip and board.
- */
-struct omap_opp {
-	bool enabled;
-	unsigned long rate;
-	u8 opp_id;
-	u16 vsel;
-};
+#include <plat/opp.h>
 
 extern struct omap_opp *mpu_opps;
 extern struct omap_opp *dsp_opps;
-- 
1.6.3.3

[PATCH 04/10 V4] omap3: pm: srf: use opp accessor functions

[Nishanth Menon]

With the accessor functions, many of the direct accesses are
redundant. However we do not want to rewrite SRF at this point of time
We do the following here:
Remove get_opp and introduce three SRF specific accessor functions:
	opp_to_freq, freq_to_opp - need this coz of usage of opp IDs
	NOTE: These functions should be removed at a later point
	of time.
get_opp is removed because, with the above functions, it is
redundant.

NOTE: this implementation is just a start and leaves scope for
further cleanups which can be added on top.

NOTE: this adds the following warnings:
arch/arm/mach-omap2/resource34xx.c: In function 'opp_to_freq':
arch/arm/mach-omap2/resource34xx.c:184: warning: 'opp_id' is deprecated (declared at arch/arm/plat-omap/include/plat/opp.h:33)
arch/arm/mach-omap2/resource34xx.c: In function 'freq_to_opp':
arch/arm/mach-omap2/resource34xx.c:213: warning: 'opp_id' is deprecated (declared at arch/arm/plat-omap/include/plat/opp.h:33)
arch/arm/mach-omap2/resource34xx.c: In function 'init_opp':
arch/arm/mach-omap2/resource34xx.c:237: warning: 'freq_to_opp' is deprecated (declared at arch/arm/mach-omap2/resource34xx.c:204)
arch/arm/mach-omap2/resource34xx.c:244: warning: 'freq_to_opp' is deprecated (declared at arch/arm/mach-omap2/resource34xx.c:204)
arch/arm/mach-omap2/resource34xx.c: In function 'program_opp_freq':
arch/arm/mach-omap2/resource34xx.c:297: warning: 'opp_to_freq' is deprecated (declared at arch/arm/mach-omap2/resource34xx.c:175)
arch/arm/mach-omap2/resource34xx.c:298: warning: 'opp_to_freq' is deprecated (declared at arch/arm/mach-omap2/resource34xx.c:175)
arch/arm/mach-omap2/resource34xx.c:303: warning: 'opp_to_freq' is deprecated (declared at arch/arm/mach-omap2/resource34xx.c:175)
arch/arm/mach-omap2/resource34xx.c: In function 'program_opp':
arch/arm/mach-omap2/resource34xx.c:346: warning: 'opp_id' is deprecated (declared at arch/arm/plat-omap/include/plat/opp.h:33)
arch/arm/mach-omap2/resource34xx.c:347: warning: 'opp_id' is deprecated (declared at arch/arm/plat-omap/include/plat/opp.h:33)
arch/arm/mach-omap2/resource34xx.c:351: warning: 'opp_to_freq' is deprecated (declared at arch/arm/mach-omap2/resource34xx.c:175)
arch/arm/mach-omap2/resource34xx.c:376: warning: 'opp_to_freq' is deprecated (declared at arch/arm/mach-omap2/resource34xx.c:175)
arch/arm/mach-omap2/resource34xx.c: In function 'resource_set_opp_level':
arch/arm/mach-omap2/resource34xx.c:414: warning: 'opp_to_freq' is deprecated (declared at arch/arm/mach-omap2/resource34xx.c:175)
arch/arm/mach-omap2/resource34xx.c:415: warning: 'opp_to_freq' is deprecated (declared at arch/arm/mach-omap2/resource34xx.c:175)
arch/arm/mach-omap2/resource34xx.c:417: warning: 'opp_to_freq' is deprecated (declared at arch/arm/mach-omap2/resource34xx.c:175)
arch/arm/mach-omap2/resource34xx.c: In function 'set_opp':
arch/arm/mach-omap2/resource34xx.c:491: warning: 'freq_to_opp' is deprecated (declared at arch/arm/mach-omap2/resource34xx.c:204)
arch/arm/mach-omap2/resource34xx.c: In function 'validate_opp':
arch/arm/mach-omap2/resource34xx.c:510: warning: 'opp_to_freq' is deprecated (declared at arch/arm/mach-omap2/resource34xx.c:175)
arch/arm/mach-omap2/resource34xx.c:512: warning: 'opp_to_freq' is deprecated (declared at arch/arm/mach-omap2/resource34xx.c:175)
arch/arm/mach-omap2/resource34xx.c: In function 'init_freq':
arch/arm/mach-omap2/resource34xx.c:535: warning: 'opp_to_freq' is deprecated (declared at arch/arm/mach-omap2/resource34xx.c:175)
arch/arm/mach-omap2/resource34xx.c:538: warning: 'opp_to_freq' is deprecated (declared at arch/arm/mach-omap2/resource34xx.c:175)
arch/arm/mach-omap2/resource34xx.c: In function 'set_freq':
arch/arm/mach-omap2/resource34xx.c:554: warning: 'freq_to_opp' is deprecated (declared at arch/arm/mach-omap2/resource34xx.c:204)
arch/arm/mach-omap2/resource34xx.c:559: warning: 'freq_to_opp' is deprecated (declared at arch/arm/mach-omap2/resource34xx.c:204)
arch/arm/mach-omap2/resource34xx.c: In function 'validate_freq':
arch/arm/mach-omap2/resource34xx.c:573: warning: 'freq_to_opp' is deprecated (declared at arch/arm/mach-omap2/resource34xx.c:204)
arch/arm/mach-omap2/resource34xx.c:575: warning: 'freq_to_opp' is deprecated (declared at arch/arm/mach-omap2/resource34xx.c:204)

Cc: Benoit Cousson <b-cousson@ti.com>
Cc: Eduardo Valentin <eduardo.valentin@nokia.com>
Cc: Kevin Hilman <khilman@deeprootsystems.com>
Cc: Madhusudhan Chikkature Rajashekar <madhu.cr@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Romit Dasgupta <romit@ti.com>
Cc: Sanjeev Premi <premi@ti.com>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Sergio Alberto Aguirre Rodriguez <saaguirre@ti.com>
Cc: Tero Kristo <Tero.Kristo@nokia.com>
Cc: Thara Gopinath <thara@ti.com>
Cc: Vishwanath Sripathy <vishwanath.bs@ti.com>

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 arch/arm/mach-omap2/resource34xx.c |  242 ++++++++++++++++++++++++++----------
 1 files changed, 176 insertions(+), 66 deletions(-)

diff --git a/arch/arm/mach-omap2/resource34xx.c b/arch/arm/mach-omap2/resource34xx.c
index af6b3c1..1f2c713 100644
--- a/arch/arm/mach-omap2/resource34xx.c
+++ b/arch/arm/mach-omap2/resource34xx.c
@@ -19,6 +19,8 @@
 #include <linux/pm_qos_params.h>
 #include <linux/cpufreq.h>
 #include <linux/delay.h>
+#include <linux/errno.h>
+#include <linux/err.h>
 
 #include <plat/powerdomain.h>
 #include <plat/clockdomain.h>
@@ -155,21 +157,61 @@ static int curr_vdd1_opp;
 static int curr_vdd2_opp;
 static DEFINE_MUTEX(dvfs_mutex);
 
-static unsigned short get_opp(struct omap_opp *opp_freq_table,
+/* Introducing deprecated function because we got to.. */
+#define IS_OPP_TERMINATOR(opps, i) (!(opps)[(i)].enabled &&	\
+		!(opps)[(i)].rate && !(opps)[(i)].vsel)
+
+/**
+ * opp_to_freq - convert OPPID to frequency (DEPRECATED)
+ * @freq: return frequency back to caller
+ * @opps: opp list
+ * @opp_id: OPP ID we are searching for
+ *
+ * return 0 and freq is populated if we find the opp_id, else,
+ * we return error
+ *
+ * NOTE: this function is a standin for the timebeing as opp_id is deprecated
+ */
+static int __deprecated opp_to_freq(unsigned long *freq,
+		const struct omap_opp *opps, u8 opp_id)
+{
+	int i = 1;
+
+	BUG_ON(!freq || !opps);
+
+	/* The first entry is a dummy one, loop till we hit terminator */
+	while (!IS_OPP_TERMINATOR(opps, i)) {
+		if (opps[i].enabled && (opps[i].opp_id == opp_id)) {
+			*freq = opps[i].rate;
+			return 0;
+		}
+		i++;
+	}
+
+	return -EINVAL;
+}
+
+/**
+ * freq_to_opp - convert a frequency back to OPP ID (DEPRECATED)
+ * @opp_id: opp ID returned back to caller
+ * @opps: opp list
+ * @freq: frequency we are searching for
+ *
+ * return 0 and opp_id is populated if we find the freq, else, we return error
+ *
+ * NOTE: this function is a standin for the timebeing as opp_id is deprecated
+ */
+static int __deprecated freq_to_opp(u8 *opp_id, struct omap_opp *opps,
 		unsigned long freq)
 {
-	struct omap_opp *prcm_config;
-	prcm_config = opp_freq_table;
-
-	if (prcm_config->rate <= freq)
-		return prcm_config->opp_id; /* Return the Highest OPP */
-	for (; prcm_config->rate; prcm_config--)
-		if (prcm_config->rate < freq)
-			return (prcm_config+1)->opp_id;
-		else if (prcm_config->rate == freq)
-			return prcm_config->opp_id;
-	/* Return the least OPP */
-	return (prcm_config+1)->opp_id;
+	struct omap_opp *opp;
+
+	BUG_ON(!opp_id || !opps);
+	opp = opp_find_freq_approx(opps, &freq, OPP_SEARCH_HIGH);
+	if (IS_ERR(opp))
+		return -EINVAL;
+	*opp_id = opp->opp_id;
+	return 0;
 }
 
 /**
@@ -178,6 +220,8 @@ static unsigned short get_opp(struct omap_opp *opp_freq_table,
 void init_opp(struct shared_resource *resp)
 {
 	struct clk *l3_clk;
+	int ret;
+	u8 opp_id;
 	resp->no_of_users = 0;
 
 	if (!mpu_opps || !dsp_opps || !l3_opps)
@@ -190,17 +234,18 @@ void init_opp(struct shared_resource *resp)
 		vdd1_resp = resp;
 		dpll1_clk = clk_get(NULL, "dpll1_ck");
 		dpll2_clk = clk_get(NULL, "dpll2_ck");
-		resp->curr_level = get_opp(mpu_opps + MAX_VDD1_OPP,
-				dpll1_clk->rate);
-		curr_vdd1_opp = resp->curr_level;
+		ret = freq_to_opp(&opp_id, mpu_opps, dpll1_clk->rate);
+		BUG_ON(ret); /* TBD Cleanup handling */
+		curr_vdd1_opp = opp_id;
 	} else if (strcmp(resp->name, "vdd2_opp") == 0) {
 		vdd2_resp = resp;
 		dpll3_clk = clk_get(NULL, "dpll3_m2_ck");
 		l3_clk = clk_get(NULL, "l3_ick");
-		resp->curr_level = get_opp(l3_opps + MAX_VDD2_OPP,
-				l3_clk->rate);
-		curr_vdd2_opp = resp->curr_level;
+		ret = freq_to_opp(&opp_id, l3_opps, l3_clk->rate);
+		BUG_ON(ret); /* TBD Cleanup handling */
+		curr_vdd2_opp = opp_id;
 	}
+	resp->curr_level = opp_id;
 	return;
 }
 
@@ -242,24 +287,40 @@ static int program_opp_freq(int res, int target_level, int current_level)
 {
 	int ret = 0, l3_div;
 	int *curr_opp;
+	unsigned long mpu_freq, dsp_freq, l3_freq;
+#ifndef CONFIG_CPU_FREQ
+	unsigned long mpu_cur_freq;
+#endif
+
+	/* Check if I can actually switch or not */
+	if (res == VDD1_OPP) {
+		ret = opp_to_freq(&mpu_freq, mpu_opps, target_level);
+		ret |= opp_to_freq(&dsp_freq, dsp_opps, target_level);
+#ifndef CONFIG_CPU_FREQ
+		ret |= opp_to_freq(&mpu_cur_freq, mpu_opps, current_level);
+#endif
+	} else {
+		ret = opp_to_freq(&l3_freq, l3_opps, target_level);
+	}
+	/* we would have caught all bad levels earlier.. */
+	if (unlikely(ret))
+		return ret;
 
 	lock_scratchpad_sem();
 	if (res == VDD1_OPP) {
 		curr_opp = &curr_vdd1_opp;
-		clk_set_rate(dpll1_clk, mpu_opps[target_level].rate);
-		clk_set_rate(dpll2_clk, dsp_opps[target_level].rate);
+		clk_set_rate(dpll1_clk, mpu_freq);
+		clk_set_rate(dpll2_clk, dsp_freq);
 #ifndef CONFIG_CPU_FREQ
 		/*Update loops_per_jiffy if processor speed is being changed*/
 		loops_per_jiffy = compute_lpj(loops_per_jiffy,
-			mpu_opps[current_level].rate/1000,
-			mpu_opps[target_level].rate/1000);
+			mpu_cur_freq / 1000, mpu_freq / 1000);
 #endif
 	} else {
 		curr_opp = &curr_vdd2_opp;
 		l3_div = cm_read_mod_reg(CORE_MOD, CM_CLKSEL) &
 			OMAP3430_CLKSEL_L3_MASK;
-		ret = clk_set_rate(dpll3_clk,
-				l3_opps[target_level].rate * l3_div);
+		ret = clk_set_rate(dpll3_clk, l3_freq * l3_div);
 	}
 	if (ret) {
 		unlock_scratchpad_sem();
@@ -278,6 +339,7 @@ static int program_opp(int res, struct omap_opp *opp, int target_level,
 		int current_level)
 {
 	int i, ret = 0, raise;
+	unsigned long freq;
 #ifdef CONFIG_OMAP_SMARTREFLEX
 	unsigned long t_opp, c_opp;
 
@@ -285,13 +347,10 @@ static int program_opp(int res, struct omap_opp *opp, int target_level,
 	c_opp = ID_VDD(res) | ID_OPP_NO(opp[current_level].opp_id);
 #endif
 
-	/* Only allow enabled OPPs */
-	if (!opp[target_level].enabled)
-		return -EINVAL;
-
-	/* Sanity check of the OPP params before attempting to set */
-	if (!opp[target_level].rate || !opp[target_level].vsel)
-		return -EINVAL;
+	/* See if have a freq associated, if not, invalid opp */
+	ret = opp_to_freq(&freq, opp, target_level);
+	if (unlikely(ret))
+		return ret;
 
 	if (target_level > current_level)
 		raise = 1;
@@ -303,10 +362,25 @@ static int program_opp(int res, struct omap_opp *opp, int target_level,
 			ret = program_opp_freq(res, target_level,
 					current_level);
 #ifdef CONFIG_OMAP_SMARTREFLEX
-		else
-			sr_voltagescale_vcbypass(t_opp, c_opp,
-				opp[target_level].vsel,
-				opp[current_level].vsel);
+		else {
+			u8 vc, vt;
+			struct omap_opp *oppx;
+			/*
+			 * transitioning from good to good OPP
+			 * none of the following should fail..
+			 */
+			oppx = opp_find_freq_exact(opp, freq, true);
+			BUG_ON(IS_ERR(oppx));
+			vt = oppx->vsel;
+
+			BUG_ON(opp_to_freq(&freq, opp, current_level));
+			oppx = opp_find_freq_exact(opp, freq, true);
+			BUG_ON(IS_ERR(oppx));
+			vc = oppx->vsel;
+
+			/* ok to scale.. */
+			sr_voltagescale_vcbypass(t_opp, c_opp, vt, vc);
+		}
 #endif
 	}
 
@@ -315,7 +389,8 @@ static int program_opp(int res, struct omap_opp *opp, int target_level,
 
 int resource_set_opp_level(int res, u32 target_level, int flags)
 {
-	unsigned long mpu_freq, mpu_old_freq;
+	unsigned long mpu_freq, mpu_old_freq, l3_freq;
+	int ret;
 #ifdef CONFIG_CPU_FREQ
 	struct cpufreq_freqs freqs_notify;
 #endif
@@ -334,6 +409,16 @@ int resource_set_opp_level(int res, u32 target_level, int flags)
 	if (!mpu_opps || !dsp_opps || !l3_opps)
 		return 0;
 
+	/* Check if I can actually switch or not */
+	if (res == VDD1_OPP) {
+		ret = opp_to_freq(&mpu_freq, mpu_opps, target_level);
+		ret |= opp_to_freq(&mpu_old_freq, mpu_opps, resp->curr_level);
+	} else {
+		ret = opp_to_freq(&l3_freq, l3_opps, target_level);
+	}
+	if (ret)
+		return ret;
+
 	mutex_lock(&dvfs_mutex);
 
 	if (res == VDD1_OPP) {
@@ -341,9 +426,6 @@ int resource_set_opp_level(int res, u32 target_level, int flags)
 			mutex_unlock(&dvfs_mutex);
 			return 0;
 		}
-		mpu_old_freq = mpu_opps[resp->curr_level].rate;
-		mpu_freq = mpu_opps[target_level].rate;
-
 #ifdef CONFIG_CPU_FREQ
 		freqs_notify.old = mpu_old_freq/1000;
 		freqs_notify.new = mpu_freq/1000;
@@ -371,15 +453,13 @@ int resource_set_opp_level(int res, u32 target_level, int flags)
 
 int set_opp(struct shared_resource *resp, u32 target_level)
 {
-	unsigned long tput;
-	unsigned long req_l3_freq;
-	int ind;
+	int ret = -EINVAL;
 
 	if (resp == vdd1_resp) {
 		if (target_level < 3)
 			resource_release("vdd2_opp", &vdd2_dev);
 
-		resource_set_opp_level(VDD1_OPP, target_level, 0);
+		ret = resource_set_opp_level(VDD1_OPP, target_level, 0);
 		/*
 		 * For VDD1 OPP3 and above, make sure the interconnect
 		 * is at 100Mhz or above.
@@ -389,21 +469,30 @@ int set_opp(struct shared_resource *resp, u32 target_level)
 			resource_request("vdd2_opp", &vdd2_dev, 400000);
 
 	} else if (resp == vdd2_resp) {
-		tput = target_level;
+		unsigned long req_l3_freq;
+		struct omap_opp *oppx = NULL;
 
 		/* Convert the tput in KiB/s to Bus frequency in MHz */
-		req_l3_freq = (tput * 1000)/4;
-
-		for (ind = 2; ind <= MAX_VDD2_OPP; ind++)
-			if ((l3_opps + ind)->rate >= req_l3_freq) {
-				target_level = ind;
-				break;
-			}
-
-		/* Set the highest OPP possible */
-		if (ind > MAX_VDD2_OPP)
-			target_level = ind-1;
-		resource_set_opp_level(VDD2_OPP, target_level, 0);
+		req_l3_freq = (target_level * 1000)/4;
+
+		/* Do I have a best match? */
+		oppx = opp_find_freq_approx(l3_opps, &req_l3_freq,
+					OPP_SEARCH_HIGH);
+		if (IS_ERR(oppx)) {
+			/* Give me the best we got */
+			req_l3_freq = ULONG_MAX;
+			oppx = opp_find_freq_approx(l3_opps,
+					&req_l3_freq, OPP_SEARCH_LOW);
+		}
+
+		/* uh uh.. no OPPs?? */
+		BUG_ON(IS_ERR(oppx));
+
+		ret = freq_to_opp((u8 *)&target_level, l3_opps, req_l3_freq);
+		/* we dont expect this to fail */
+		BUG_ON(ret);
+
+		ret = resource_set_opp_level(VDD2_OPP, target_level, 0);
 	}
 	return 0;
 }
@@ -416,6 +505,11 @@ int set_opp(struct shared_resource *resp, u32 target_level)
  */
 int validate_opp(struct shared_resource *resp, u32 target_level)
 {
+	unsigned long x;
+	if (strcmp(resp->name, "mpu_freq") == 0)
+		return opp_to_freq(&x, mpu_opps, target_level);
+	else if (strcmp(resp->name, "dsp_freq") == 0)
+		return opp_to_freq(&x, dsp_opps, target_level);
 	return 0;
 }
 
@@ -425,6 +519,8 @@ int validate_opp(struct shared_resource *resp, u32 target_level)
 void init_freq(struct shared_resource *resp)
 {
 	char *linked_res_name;
+	int ret = -EINVAL;
+	unsigned long freq;
 	resp->no_of_users = 0;
 
 	if (!mpu_opps || !dsp_opps)
@@ -436,32 +532,46 @@ void init_freq(struct shared_resource *resp)
 	*/
 	if (strcmp(resp->name, "mpu_freq") == 0)
 		/* MPU freq in Mhz */
-		resp->curr_level = mpu_opps[curr_vdd1_opp].rate;
+		ret = opp_to_freq(&freq, mpu_opps, curr_vdd1_opp);
 	else if (strcmp(resp->name, "dsp_freq") == 0)
 		/* DSP freq in Mhz */
-		resp->curr_level = dsp_opps[curr_vdd1_opp].rate;
+		ret = opp_to_freq(&freq, dsp_opps, curr_vdd1_opp);
+	BUG_ON(ret);
+
+	resp->curr_level = freq;
 	return;
 }
 
 int set_freq(struct shared_resource *resp, u32 target_level)
 {
-	unsigned int vdd1_opp;
+	u8 vdd1_opp;
+	int ret = -EINVAL;
 
 	if (!mpu_opps || !dsp_opps)
 		return 0;
 
 	if (strcmp(resp->name, "mpu_freq") == 0) {
-		vdd1_opp = get_opp(mpu_opps + MAX_VDD1_OPP, target_level);
-		resource_request("vdd1_opp", &dummy_mpu_dev, vdd1_opp);
+		ret = freq_to_opp(&vdd1_opp, mpu_opps, target_level);
+		if (!ret)
+			ret = resource_request("vdd1_opp", &dummy_mpu_dev,
+					vdd1_opp);
 	} else if (strcmp(resp->name, "dsp_freq") == 0) {
-		vdd1_opp = get_opp(dsp_opps + MAX_VDD1_OPP, target_level);
-		resource_request("vdd1_opp", &dummy_dsp_dev, vdd1_opp);
+		ret = freq_to_opp(&vdd1_opp, dsp_opps, target_level);
+		if (!ret)
+			ret = resource_request("vdd1_opp", &dummy_dsp_dev,
+					vdd1_opp);
 	}
-	resp->curr_level = target_level;
-	return 0;
+	if (!ret)
+		resp->curr_level = target_level;
+	return ret;
 }
 
 int validate_freq(struct shared_resource *resp, u32 target_level)
 {
+	u8 x;
+	if (strcmp(resp->name, "mpu_freq") == 0)
+		return freq_to_opp(&x, mpu_opps, target_level);
+	else if (strcmp(resp->name, "dsp_freq") == 0)
+		return freq_to_opp(&x, dsp_opps, target_level);
 	return 0;
 }
-- 
1.6.3.3

[PATCH 05/10 V4] omap3: pm: sr: replace get_opp with freq_to_opp

[Nishanth Menon]

SmartReflex implements a get_opp to search through the opp table,
replace it opp_is_valid to verify the OPPs and getting the opp_id.

NOTE: usage of opp_id is deprecated and this introduces the following
warnings:
arch/arm/mach-omap2/smartreflex.c: In function 'get_vdd1_opp':
arch/arm/mach-omap2/smartreflex.c:161: warning: 'opp_id' is deprecated (declared at arch/arm/plat-omap/include/plat/opp.h:33)
arch/arm/mach-omap2/smartreflex.c: In function 'get_vdd2_opp':
arch/arm/mach-omap2/smartreflex.c:176: warning: 'opp_id' is deprecated (declared at arch/arm/plat-omap/include/plat/opp.h:33)

Cc: Benoit Cousson <b-cousson@ti.com>
Cc: Eduardo Valentin <eduardo.valentin@nokia.com>
Cc: Kevin Hilman <khilman@deeprootsystems.com>
Cc: Madhusudhan Chikkature Rajashekar <madhu.cr@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Romit Dasgupta <romit@ti.com>
Cc: Sanjeev Premi <premi@ti.com>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Sergio Alberto Aguirre Rodriguez <saaguirre@ti.com>
Cc: Tero Kristo <Tero.Kristo@nokia.com>
Cc: Thara Gopinath <thara@ti.com>
Cc: Vishwanath Sripathy <vishwanath.bs@ti.com>

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 arch/arm/mach-omap2/smartreflex.c |   44 +++++++++++-------------------------
 1 files changed, 14 insertions(+), 30 deletions(-)

diff --git a/arch/arm/mach-omap2/smartreflex.c b/arch/arm/mach-omap2/smartreflex.c
index be3a1da..ccc98a0 100644
--- a/arch/arm/mach-omap2/smartreflex.c
+++ b/arch/arm/mach-omap2/smartreflex.c
@@ -146,50 +146,34 @@ static u32 cal_test_nvalue(u32 sennval, u32 senpval)
 		(rnsenn << NVALUERECIPROCAL_RNSENN_SHIFT);
 }
 
-/* determine the current OPP from the frequency
- * we need to give this function last element of OPP rate table
- * and the frequency
- */
-static u16 get_opp(struct omap_opp *opp_freq_table,
-					unsigned long freq)
-{
-	struct omap_opp *prcm_config;
-
-	prcm_config = opp_freq_table;
-
-	if (prcm_config->rate <= freq)
-		return prcm_config->opp_id; /* Return the Highest OPP */
-	for (; prcm_config->rate; prcm_config--)
-		if (prcm_config->rate < freq)
-			return (prcm_config+1)->opp_id;
-		else if (prcm_config->rate == freq)
-			return prcm_config->opp_id;
-	/* Return the least OPP */
-	return (prcm_config+1)->opp_id;
-}
-
-static u16 get_vdd1_opp(void)
+static u8 get_vdd1_opp(void)
 {
-	u16 opp;
+	struct omap_opp *opp;
 
 	if (sr1.vdd_opp_clk == NULL || IS_ERR(sr1.vdd_opp_clk) ||
 							mpu_opps == NULL)
 		return 0;
 
-	opp = get_opp(mpu_opps + MAX_VDD1_OPP, sr1.vdd_opp_clk->rate);
-	return opp;
+	opp = opp_find_freq_exact(mpu_opps, sr1.vdd_opp_clk->rate, true);
+	if (IS_ERR(opp))
+		return 0;
+
+	return opp->opp_id;
 }
 
-static u16 get_vdd2_opp(void)
+static u8 get_vdd2_opp(void)
 {
-	u16 opp;
+	struct omap_opp *opp;
 
 	if (sr2.vdd_opp_clk == NULL || IS_ERR(sr2.vdd_opp_clk) ||
 							l3_opps == NULL)
 		return 0;
 
-	opp = get_opp(l3_opps + MAX_VDD2_OPP, sr2.vdd_opp_clk->rate);
-	return opp;
+	opp = opp_find_freq_exact(l3_opps, sr2.vdd_opp_clk->rate, true);
+	if (IS_ERR(opp))
+		return 0;
+
+	return opp->opp_id;
 }
 
 
-- 
1.6.3.3

[PATCH 06/10 V4] omap3: pm: use opp accessor functions for omap-target

[Nishanth Menon]

The logic in omap-target can now be improved with the accessor
functions. Dont scan through the list manually, instead use
get_next_freq to do the scanning.

Cc: Benoit Cousson <b-cousson@ti.com>
Cc: Eduardo Valentin <eduardo.valentin@nokia.com>
Cc: Kevin Hilman <khilman@deeprootsystems.com>
Cc: Madhusudhan Chikkature Rajashekar <madhu.cr@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Romit Dasgupta <romit@ti.com>
Cc: Sanjeev Premi <premi@ti.com>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Sergio Alberto Aguirre Rodriguez <saaguirre@ti.com>
Cc: Tero Kristo <Tero.Kristo@nokia.com>
Cc: Thara Gopinath <thara@ti.com>
Cc: Vishwanath Sripathy <vishwanath.bs@ti.com>

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 arch/arm/plat-omap/cpu-omap.c |   12 ++++--------
 1 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/arch/arm/plat-omap/cpu-omap.c b/arch/arm/plat-omap/cpu-omap.c
index 449b6b6..43d3da5 100644
--- a/arch/arm/plat-omap/cpu-omap.c
+++ b/arch/arm/plat-omap/cpu-omap.c
@@ -111,14 +111,10 @@ static int omap_target(struct cpufreq_policy *policy,
 	cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
 #elif defined(CONFIG_ARCH_OMAP3) && !defined(CONFIG_OMAP_PM_NONE)
 	if (mpu_opps) {
-		int ind;
-		for (ind = 1; ind <= MAX_VDD1_OPP; ind++) {
-			if (mpu_opps[ind].rate/1000 >= target_freq) {
-				omap_pm_cpu_set_freq
-					(mpu_opps[ind].rate);
-				break;
-			}
-		}
+		unsigned long freq = target_freq * 1000;
+		if (!IS_ERR(opp_find_freq_approx(mpu_opps, &freq,
+					OPP_SEARCH_HIGH)))
+			omap_pm_cpu_set_freq(freq);
 	}
 #endif
 	return ret;
-- 
1.6.3.3

[PATCH 07/10 V4] omap3: clk: use pm accessor functions for cpufreq table

[Nishanth Menon]

omap2_clk_init_cpufreq_table currently directly accesses the opp
table, making it unscalable to various OMAPs. Instead use the
accessor functions to populate the cpufreq table.

includes fixes from comment:
http://marc.info/?l=linux-omap&m=126027057909254&w=2

Cc: Benoit Cousson <b-cousson@ti.com>
Cc: Eduardo Valentin <eduardo.valentin@nokia.com>
Cc: Kevin Hilman <khilman@deeprootsystems.com>
Cc: Madhusudhan Chikkature Rajashekar <madhu.cr@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Romit Dasgupta <romit@ti.com>
Cc: Sanjeev Premi <premi@ti.com>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Sergio Alberto Aguirre Rodriguez <saaguirre@ti.com>
Cc: Tero Kristo <Tero.Kristo@nokia.com>
Cc: Thara Gopinath <thara@ti.com>
Cc: Vishwanath Sripathy <vishwanath.bs@ti.com>

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 arch/arm/mach-omap2/clock34xx.c |   40 ++++++++++++++++++++++++++------------
 1 files changed, 27 insertions(+), 13 deletions(-)

diff --git a/arch/arm/mach-omap2/clock34xx.c b/arch/arm/mach-omap2/clock34xx.c
index 5150939..6c4a609 100644
--- a/arch/arm/mach-omap2/clock34xx.c
+++ b/arch/arm/mach-omap2/clock34xx.c
@@ -1042,30 +1042,44 @@ static unsigned long omap3_clkoutx2_recalc(struct clk *clk)
 #if defined(CONFIG_ARCH_OMAP3)
 
 #ifdef CONFIG_CPU_FREQ
-static struct cpufreq_frequency_table freq_table[MAX_VDD1_OPP+1];
+
+static struct cpufreq_frequency_table *freq_table;
 
 void omap2_clk_init_cpufreq_table(struct cpufreq_frequency_table **table)
 {
-	struct omap_opp *prcm;
 	int i = 0;
+	int opp_num;
+	struct omap_opp *opp = mpu_opps;
+	unsigned long freq = ULONG_MAX;
 
-	if (!mpu_opps)
+	if (!mpu_opps) {
+		pr_warning("%s: failed to initialize frequency"
+				"table\n", __func__);
+		return;
+	}
+	opp_num = opp_get_opp_count(mpu_opps);
+	if (opp_num < 0) {
+		pr_err("%s: no opp table?\n", __func__);
 		return;
-
-	prcm = mpu_opps + MAX_VDD1_OPP;
-	for (; prcm->rate; prcm--) {
-		freq_table[i].index = i;
-		freq_table[i].frequency = prcm->rate / 1000;
-		i++;
 	}
 
-	if (i == 0) {
-		printk(KERN_WARNING "%s: failed to initialize frequency \
-								table\n",
-								__func__);
+	freq_table = kmalloc(sizeof(struct cpufreq_frequency_table) *
+			(opp_num + 1), GFP_ATOMIC);
+	if (!freq_table) {
+		pr_warning("%s: failed to allocate frequency"
+				"table\n", __func__);
 		return;
 	}
 
+	while (!IS_ERR(opp = opp_find_freq_approx(opp, &freq,
+					OPP_SEARCH_LOW))) {
+		freq_table[i].index = i;
+		freq_table[i].frequency = freq / 1000;
+		i++;
+		/* set the next benchmark to search */
+		freq--;
+	}
+
 	freq_table[i].index = i;
 	freq_table[i].frequency = CPUFREQ_TABLE_END;
 
-- 
1.6.3.3

[PATCH 08/10] omap3: pm: remove VDDx_MIN/MAX macros

[Nishanth Menon]

Since accessor functions are used through out, we dont depend
on the predefined macros to know the limits of the opp table.
Hence, remove these.

Cc: Benoit Cousson <b-cousson@ti.com>
Cc: Eduardo Valentin <eduardo.valentin@nokia.com>
Cc: Kevin Hilman <khilman@deeprootsystems.com>
Cc: Madhusudhan Chikkature Rajashekar <madhu.cr@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Romit Dasgupta <romit@ti.com>
Cc: Sanjeev Premi <premi@ti.com>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Sergio Alberto Aguirre Rodriguez <saaguirre@ti.com>
Cc: Tero Kristo <Tero.Kristo@nokia.com>
Cc: Thara Gopinath <thara@ti.com>
Cc: Vishwanath Sripathy <vishwanath.bs@ti.com>

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 arch/arm/plat-omap/include/plat/omap34xx.h |    5 -----
 1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/arch/arm/plat-omap/include/plat/omap34xx.h b/arch/arm/plat-omap/include/plat/omap34xx.h
index 6b849e5..9768f8a 100644
--- a/arch/arm/plat-omap/include/plat/omap34xx.h
+++ b/arch/arm/plat-omap/include/plat/omap34xx.h
@@ -98,10 +98,5 @@
 #define VDD2_OPP2	0x2
 #define VDD2_OPP3	0x3
 
-#define MIN_VDD1_OPP	VDD1_OPP1
-#define MAX_VDD1_OPP	VDD1_OPP5
-#define MIN_VDD2_OPP	VDD2_OPP1
-#define MAX_VDD2_OPP	VDD2_OPP3
-
 #endif /* __ASM_ARCH_OMAP34XX_H */
 
-- 
1.6.3.3

[PATCH 09/10 V4] omap3: pm: introduce 3630 opps

[Nishanth Menon]

Introduce the OMAP3630 OPPs including the defined OPP tuples.

Further information on OMAP3630 can be found here:
http://focus.ti.com/general/docs/wtbu/wtbuproductcontent.tsp?templateId=6123&navigationId=12836&contentId=52606

OMAP36xx family introduces:
VDD1 with 4 OPPs, of which OPP3 & 4 are available only on devices yet
to be introduced in 36xx family. Meanwhile, VDD2 has 2 opps.

Device Support of OPPs->
           |<-3630-600->| (default)
           |<-      3630-800    ->| (device: TBD)
           |<-      3630-1000            ->| (device: TBD)
H/w OPP-> OPP50 OPP100       OPP-Turbo   OPP1G-SB
VDD1      OPP1  OPP2         OPP3        OPP4
VDD2      OPP1  OPP2         OPP2        OPP2

Note:
a) TI h/w naming for OPPs are now standardized in terms of OPP50, 100,
   Turbo and SB. This maps as shown above to the opp IDs (s/w term).
b) For boards which need custom VDD1/2 OPPs, the opp table can be
   updated by the board file on a need basis after the
   omap3_pm_init_opp_table call. The OPPs introduced here are the
   official OPP table at this point in time.

Cc: Benoit Cousson <b-cousson@ti.com>
Cc: Eduardo Valentin <eduardo.valentin@nokia.com>
Cc: Kevin Hilman <khilman@deeprootsystems.com>
Cc: Madhusudhan Chikkature Rajashekar <madhu.cr@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Romit Dasgupta <romit@ti.com>
Cc: Sanjeev Premi <premi@ti.com>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Sergio Alberto Aguirre Rodriguez <saaguirre@ti.com>
Cc: Tero Kristo <Tero.Kristo@nokia.com>
Cc: Thara Gopinath <thara@ti.com>
Cc: Vishwanath Sripathy <vishwanath.bs@ti.com>

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 arch/arm/mach-omap2/pm34xx.c |   43 +++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 42 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index e40a036..e5fa5bf 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -140,6 +140,38 @@ static struct omap_opp_def __initdata omap34xx_dsp_rate_table[] = {
 	OMAP_OPP_DEF_TERMINATOR
 };
 
+static struct omap_opp_def __initdata omap36xx_mpu_rate_table[] = {
+	/* OPP1 - OPP50 */
+	OMAP_OPP_DEF(true,  300000000, 930000),
+	/* OPP2 - OPP100 */
+	OMAP_OPP_DEF(true,  600000000, 1100000),
+	/* OPP3 - OPP-Turbo */
+	OMAP_OPP_DEF(false, 800000000, 1260000),
+	/* OPP4 - OPP-SB */
+	OMAP_OPP_DEF(false, 1000000000, 1310000),
+	OMAP_OPP_DEF_TERMINATOR
+};
+
+static struct omap_opp_def __initdata omap36xx_l3_rate_table[] = {
+	/* OPP1 - OPP50 */
+	OMAP_OPP_DEF(true, 100000000, 930000),
+	/* OPP2 - OPP100, OPP-Turbo, OPP-SB */
+	OMAP_OPP_DEF(true, 200000000, 1137500),
+	OMAP_OPP_DEF_TERMINATOR
+};
+
+static struct omap_opp_def __initdata omap36xx_dsp_rate_table[] = {
+	/* OPP1 - OPP50 */
+	OMAP_OPP_DEF(true,  260000000, 930000),
+	/* OPP2 - OPP100 */
+	OMAP_OPP_DEF(true,  520000000, 1100000),
+	/* OPP3 - OPP-Turbo */
+	OMAP_OPP_DEF(false, 660000000, 1260000),
+	/* OPP4 - OPP-SB */
+	OMAP_OPP_DEF(false, 875000000, 1310000),
+	OMAP_OPP_DEF_TERMINATOR
+};
+
 /* OMAP3 Rate Table */
 struct omap_opp *omap3_mpu_rate_table;
 struct omap_opp *omap3_dsp_rate_table;
@@ -1296,18 +1328,27 @@ static void __init configure_vc(void)
 void __init omap3_pm_init_opp_table(void)
 {
 	int i;
+	struct omap_opp_def **omap3_opp_def_list;
 	struct omap_opp_def *omap34xx_opp_def_list[] = {
 		omap34xx_mpu_rate_table,
 		omap34xx_l3_rate_table,
 		omap34xx_dsp_rate_table
 	};
+	struct omap_opp_def *omap36xx_opp_def_list[] = {
+		omap36xx_mpu_rate_table,
+		omap36xx_l3_rate_table,
+		omap36xx_dsp_rate_table
+	};
 	struct omap_opp **omap3_rate_tables[] = {
 		&omap3_mpu_rate_table,
 		&omap3_l3_rate_table,
 		&omap3_dsp_rate_table
 	};
+
+	omap3_opp_def_list = cpu_is_omap3630() ? omap36xx_opp_def_list :
+				omap34xx_opp_def_list;
 	for (i = 0; i < ARRAY_SIZE(omap3_rate_tables); i++) {
-		*omap3_rate_tables[i] = opp_init_list(omap34xx_opp_def_list[i]);
+		*omap3_rate_tables[i] = opp_init_list(omap3_opp_def_list[i]);
 		/* We dont want half configured system at the moment */
 		BUG_ON(IS_ERR(omap3_rate_tables[i]));
 	}
-- 
1.6.3.3

[PATCH 10/10] omap3: pm: omap3630 boards: enable 3630 opp tables

[Nishanth Menon]

Enable the OPP tables for 3630 platforms: SDP3630, zoom3.
NOTE: This needs the corresponding clockframework changes.

Currently this dumps the following on OPP transitions on SDP3630:
[<c002d734>] (dump_backtrace+0x0/0x110) from [<c02c9698>] (dump_stack+0x18/0x1c)
r7:00000000 r6:c003bff8 r5:c034a227 r4:00000309
[<c02c9680>] (dump_stack+0x0/0x1c) from [<c0052f24>] (warn_slowpath_common+0x50/0x68)
[<c0052ed4>] (warn_slowpath_common+0x0/0x68) from [<c0052f54>] (warn_slowpath_null+0x18/0x1c)
r7:c7801000 r6:0f7f4900 r5:80000013 r4:00000000
[<c0052f3c>] (warn_slowpath_null+0x0/0x1c) from [<c003bff8>] (omap3_noncore_dpll_set_rate+0x294/0x2d8)
[<c003bd64>] (omap3_noncore_dpll_set_rate+0x0/0x2d8) from [<c0036e24>] (omap2_clk_set_rate+0x28/0x34)
[<c0036dfc>] (omap2_clk_set_rate+0x0/0x34) from [<c003ecec>] (clk_set_rate+0x54/0xb8)
[<c003ec98>] (clk_set_rate+0x0/0xb8) from [<c003c750>] (program_opp+0x120/0x228)
r7:c7801000 r6:00000000 r5:c03c9fe0 r4:c03c9fd8
[<c003c630>] (program_opp+0x0/0x228) from [<c003c990>] (resource_set_opp_level+0x138/0x1b0)
[<c003c858>] (resource_set_opp_level+0x0/0x1b0) from [<c0038534>] (vdd_opp_store+0x148/0x17c)
[<c00383ec>] (vdd_opp_store+0x0/0x17c) from [<c01b39dc>] (kobj_attr_store+0x20/0x24)
r7:c7833740 r6:c794bec0 r5:c78c8960 r4:00000001
[<c01b39bc>] (kobj_attr_store+0x0/0x24) from [<c0102c3c>] (sysfs_write_file+0x110/0x144)
[<c0102b2c>] (sysfs_write_file+0x0/0x144) from [<c00b2f90>] (vfs_write+0xb8/0x164)
[<c00b2ed8>] (vfs_write+0x0/0x164) from [<c00b3100>] (sys_write+0x44/0x70)
r8:40000000 r7:00000001 r6:c783de00 r5:00000000 r4:00000000
[<c00b30bc>] (sys_write+0x0/0x70) from [<c0029f80>] (ret_fast_syscall+0x0/0x2c)
r8:c002a104 r7:00000004 r6:001d7918 r5:40000000 r4:00000001

Cc: Benoit Cousson <b-cousson@ti.com>
Cc: Eduardo Valentin <eduardo.valentin@nokia.com>
Cc: Kevin Hilman <khilman@deeprootsystems.com>
Cc: Madhusudhan Chikkature Rajashekar <madhu.cr@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Romit Dasgupta <romit@ti.com>
Cc: Sanjeev Premi <premi@ti.com>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Sergio Alberto Aguirre Rodriguez <saaguirre@ti.com>
Cc: Tero Kristo <Tero.Kristo@nokia.com>
Cc: Thara Gopinath <thara@ti.com>
Cc: Vishwanath Sripathy <vishwanath.bs@ti.com>

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 arch/arm/mach-omap2/board-3630sdp.c |    9 ++++++++-
 arch/arm/mach-omap2/board-zoom3.c   |    9 ++++++++-
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/board-3630sdp.c b/arch/arm/mach-omap2/board-3630sdp.c
index 185f48d..7a429db 100755
--- a/arch/arm/mach-omap2/board-3630sdp.c
+++ b/arch/arm/mach-omap2/board-3630sdp.c
@@ -24,6 +24,8 @@
 #include <mach/board-zoom.h>
 
 #include "sdram-hynix-h8mbx00u0mer-0em.h"
+#include "pm.h"
+#include "omap3-opp.h"
 
 #if defined(CONFIG_SMC91X) || defined(CONFIG_SMC91X_MODULE)
 
@@ -76,9 +78,14 @@ static void __init omap_sdp_init_irq(void)
 {
 	omap_board_config = sdp_config;
 	omap_board_config_size = ARRAY_SIZE(sdp_config);
+
+	omap3_pm_init_opp_table();
+	/* TODO: Add RET, OFF, cpu_idle params */
+
 	omap2_init_common_hw(h8mbx00u0mer0em_sdrc_params,
 			     h8mbx00u0mer0em_sdrc_params,
-                             NULL, NULL, NULL);
+			     omap3_mpu_rate_table, omap3_dsp_rate_table,
+			     omap3_l3_rate_table);
 	omap_init_irq();
 	omap_gpio_init();
 }
diff --git a/arch/arm/mach-omap2/board-zoom3.c b/arch/arm/mach-omap2/board-zoom3.c
index fe97324..0574811 100644
--- a/arch/arm/mach-omap2/board-zoom3.c
+++ b/arch/arm/mach-omap2/board-zoom3.c
@@ -22,6 +22,8 @@
 #include <plat/board.h>
 
 #include "sdram-hynix-h8mbx00u0mer-0em.h"
+#include "pm.h"
+#include "omap3-opp.h"
 
 static void __init omap_zoom_map_io(void)
 {
@@ -36,9 +38,14 @@ static void __init omap_zoom_init_irq(void)
 {
 	omap_board_config = zoom_config;
 	omap_board_config_size = ARRAY_SIZE(zoom_config);
+
+	omap3_pm_init_opp_table();
+	/* TODO: Add RET, OFF, cpu_idle params */
+
 	omap2_init_common_hw(h8mbx00u0mer0em_sdrc_params,
 			     h8mbx00u0mer0em_sdrc_params,
-			     NULL, NULL, NULL);
+			     omap3_mpu_rate_table, omap3_dsp_rate_table,
+			     omap3_l3_rate_table);
 	omap_init_irq();
 	omap_gpio_init();
 }
-- 
1.6.3.3

Re: [PATCH 02/10 V4] omap3: pm: introduce opp accessor functions

[Kevin Hilman]

Nishanth Menon <nm@ti.com> writes:

> Modifies the initial patch From Sanjeev:
> http://patchwork.kernel.org/patch/50998/
> Discussions and comments from:
> http://marc.info/?l=linux-omap&m=125482970102327&w=2
> http://marc.info/?t=125809247500002&r=1&w=2
> http://marc.info/?l=linux-omap&m=126025973426007&w=2
> incorporated.
>
> OMAP SOCs have a standard set of tuples consisting of frequency and
> voltage pairs that the device will support per voltage domain. This
> is called Operating Points or OPP. The actual definitions of OMAP
> Operating Points varies over silicon within the same family of
> devices. For a specific domain, you can have a set of
> {frequency, voltage} pairs. As the kernel boots and more information
> is available, a set of these are activated based on the precise
> nature of device the kernel boots up on. It is interesting to
> remember that each IP which belongs to a voltage domain may define
> their own set of OPPs on top of this.
>
> This introduces a common handling OPP mechanism accross all OMAPs.
> As a start this is introduced for OMAP3 and intends to replace
> current OMAP3 opp handling mechanism.
>
> Note:
> fields of struct omap_opp is currently exposed due to the necessity
> that SRF and SR logic directly indexes the structure array fields.
> The goal however, is to make the direct usage of omap_opp deprecated
> and move to using these accessor functions. The usage in SRF and SR
> indexes based on opp_id and hence opp_id is marked deprecated to
> generate build warnings at least. Further, this usage necessitates
> need of terminator entries at the start and end of opp_* tables which
> are dynamically allocated.
>
> The accessor function definitions were collaborated with Kevin, and
> doing justice here, this implementation could not go with some of
> the better suggestions from kevin due to constraint imposed by SRF
> and SR. A better and more optimal implementation is definitely
> possible once SRF and SR are cleanedup/replaced.
>
> NOTE: OPP is a concept that can be used in all OMAPs, it is hence
> introduced under plat-omap
>
> Introduces warning:
> arch/arm/plat-omap/opp.c: In function 'opp_add':
> arch/arm/plat-omap/opp.c:191: warning: 'opp_id' is deprecated (declared at arch/arm/plat-omap/include/plat/opp.h:33)
> arch/arm/plat-omap/opp.c:199: warning: 'opp_id' is deprecated (declared at arch/arm/plat-omap/include/plat/opp.h:33)
> arch/arm/plat-omap/opp.c: In function 'opp_init_list':
> arch/arm/plat-omap/opp.c:240: warning: 'opp_id' is deprecated (declared at arch/arm/plat-omap/include/plat/opp.h:33)
>
> Cc: Benoit Cousson <b-cousson@ti.com>
> Cc: Eduardo Valentin <eduardo.valentin@nokia.com>
> Cc: Madhusudhan Chikkature Rajashekar <madhu.cr@ti.com>
> Cc: Paul Walmsley <paul@pwsan.com>
> Cc: Romit Dasgupta <romit@ti.com>
> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Cc: Sergio Alberto Aguirre Rodriguez <saaguirre@ti.com>
> Cc: Tero Kristo <Tero.Kristo@nokia.com>
> Cc: Thara Gopinath <thara@ti.com>
> Cc: Vishwanath Sripathy <vishwanath.bs@ti.com>
>
> Signed-off-by: Sanjeev Premi <premi@ti.com>
> Signed-off-by: Kevin Hilman <khilman@deeprootsystems.com>
> Signed-off-by: Nishanth Menon <nm@ti.com>
> ---
> Kevin,
> I have put your Signed-off-by to ack your contribs. pending
> formal confirmation ofcourse.

ok

>  arch/arm/plat-omap/Makefile           |    3 +
>  arch/arm/plat-omap/include/plat/opp.h |  230 ++++++++++++++++++++++++++++
>  arch/arm/plat-omap/opp.c              |  271 +++++++++++++++++++++++++++++++++
>  3 files changed, 504 insertions(+), 0 deletions(-)
>  create mode 100644 arch/arm/plat-omap/include/plat/opp.h
>  create mode 100644 arch/arm/plat-omap/opp.c
>
> diff --git a/arch/arm/plat-omap/Makefile b/arch/arm/plat-omap/Makefile
> index 95f8413..e9cf601 100644
> --- a/arch/arm/plat-omap/Makefile
> +++ b/arch/arm/plat-omap/Makefile
> @@ -12,6 +12,9 @@ obj-  :=
>  # OCPI interconnect support for 1710, 1610 and 5912
>  obj-$(CONFIG_ARCH_OMAP16XX) += ocpi.o
>  
> +# OPP support in (OMAP3+ only at the moment)
> +obj-$(CONFIG_ARCH_OMAP3) += opp.o
> +
>  # omap_device support (OMAP2+ only at the moment)
>  obj-$(CONFIG_ARCH_OMAP2) += omap_device.o
>  obj-$(CONFIG_ARCH_OMAP3) += omap_device.o
> diff --git a/arch/arm/plat-omap/include/plat/opp.h b/arch/arm/plat-omap/include/plat/opp.h
> new file mode 100644
> index 0000000..341c02b
> --- /dev/null
> +++ b/arch/arm/plat-omap/include/plat/opp.h
> @@ -0,0 +1,230 @@
> +/*
> + * OMAP OPP Interface
> + *
> + * Copyright (C) 2009 Texas Instruments Incorporated.
> + *	Nishanth Menon
> + * Copyright (C) 2009 Deep Root Systems, LLC.
> + *	Kevin Hilman
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#ifndef __ASM_ARM_OMAP_OPP_H
> +#define __ASM_ARM_OMAP_OPP_H
> +
> +/**
> + * struct omap_opp - OMAP OPP description structure
> + * @enabled:	true/false - marking this OPP as enabled/disabled
> + * @rate:	Frequency in hertz
> + * @opp_id:	(DEPRECATED) opp identifier
> + * @vsel:	Voltage in volt processor level(this usage is
> + *		DEPRECATED to use Voltage in microvolts in future)
> + *		uV = ((vsel * 12.5) + 600) * 1000
> + *
> + * This structure stores the OPP information for a given domain.
> + * Due to legacy reasons, this structure is currently exposed and
> + * will soon be removed elsewhere and will only be used as a handle
> + * from the OPP internal referencing mechanism
> + */
> +struct omap_opp {
> +	bool enabled;
> +	unsigned long rate;
> +	u8 opp_id __deprecated;
> +	u16 vsel;

How about we add 'u32 voltage' here and mark vsel as __deprecated.  Then
we no longer need both an 'struct omap_opp' and a 'struct omap_opp_def'.

Or even better, with the uv <--> vsel conversion macros you added,
couldn't we alrady define the OPPs in terms of voltage, and drop the
vsel already?

> +};
> +
> +/**
> + * opp_get_voltage() - Gets the voltage corresponding to an opp
> + * @opp:	opp for which voltage has to be returned for
> + *
> + * Return voltage in micro volt corresponding to the opp, else
> + * return 0
> + */
> +unsigned long opp_get_voltage(const struct omap_opp *opp);

ack

> +/**
> + * opp_get_freq() - Gets the frequency corresponding to an opp
> + * @opp:	opp for which frequency has to be returned for
> + *
> + * Return frequency in hertz corresponding to the opp, else
> + * return 0
> + */
> +unsigned long opp_get_freq(const struct omap_opp *opp);

ack

> +/**
> + * opp_get_opp_count() - Get number of opps enabled in the opp list
> + * @num:	returns the number of opps
> + * @oppl:	opp list
> + *
> + * This functions returns the number of opps if there are any OPPs enabled,
> + * else returns corresponding error value.
> + */
> +int opp_get_opp_count(const struct omap_opp *oppl);

ack

> +/**
> + * opp_find_freq_exact() - search for an exact frequency
> + * @oppl:	OPP list
> + * @freq:	frequency to search for
> + * @enabled:	enabled/disabled OPP to search for
> + *
> + * searches for the match in the opp list and returns handle to the matching
> + * opp if found, else returns ERR_PTR in case of error and should be handled
> + * using IS_ERR.
> + *
> + * Note enabled is a modifier for the search. if enabled=true, then the match is
> + * for exact matching frequency and is enabled. if true, the match is for exact
> + * frequency which is disabled.
> + */
> +struct omap_opp *opp_find_freq_exact(struct omap_opp *oppl,
> +				     unsigned long freq, bool enabled);

ack

I think we could drop the _exact, and just call it opp_find_freq(), but I'm
ok either way.

> +#define OPP_SEARCH_HIGH		(0 << 1)
> +#define OPP_SEARCH_LOW		(1 << 1)
> +/**
> + * opp_find_freq_approx() - Search for an rounded freq
> + * @oppl:	Starting list
> + * @freq:	Start frequency
> + * @dir_flag:	Search direction
> + *		OPP_SEARCH_HIGH - search for next highest freq
> + *		OPP_SEARCH_LOW - search for next lowest freq
> + *
> + * Search for the higher/lower *enabled* OPP from a starting freq
> + * from a start opp list.
> + *
> + * Returns *opp and *freq is populated with the next match,
> + * else returns NULL
> + * opp if found, else returns ERR_PTR in case of error.
> + *
> + * Example usages:
> + *	* find match/next highest available frequency
> + *	freq = 350000;
> + *	opp = opp_find_freq_approx(oppl, &freq, OPP_SEARCH_HIGH)))
> + *	if (IS_ERR(opp))
> + *		pr_err ("unable to find a higher frequency\n");
> + *	else
> + *		pr_info("match freq = %ld\n", freq);
> + *
> + *	* find match/next lowest available frequency
> + *	freq = 350000;
> + *	opp = opp_find_freq_approx(oppl, &freq, OPP_SEARCH_LOW)))
> + *	if (IS_ERR(opp))
> + *		pr_err ("unable to find a lower frequency\n");
> + *	else
> + *		pr_info("match freq = %ld\n", freq);
> + *
> + *	* print all supported frequencies in descending order *
> + *	opp = oppl;
> + *	freq = ULONG_MAX;
> + *	while (!IS_ERR(opp = opp_find_freq_approx(opp, &freq,
> + *		OPP_SEARCH_LOW))) {
> + *		pr_info("freq = %ld\n", freq);
> + *		freq--; * for next lower match *
> + *	}
> + *
> + *	* print all supported frequencies in ascending order *
> + *	opp = oppl;
> + *	freq = 0;
> + *	while (!IS_ERR(opp = opp_find_freq_approx(opp, &freq,
> + *			OPP_SEARCH_HIGH))) {
> + *		pr_info("freq = %ld\n", freq);
> + *		freq++; * for next higher match *
> + *	}
> + *
> + * NOTE: if we set freq as ULONG_MAX and search low, we get the highest enabled
> + * frequency
> + */
> +struct omap_opp *opp_find_freq_approx(struct omap_opp *oppl,
> +				      unsigned long *freq, u8 dir_flag);

ack

> +/**
> + * struct omap_opp_def - OMAP OPP Definition
> + * @enabled:	True/false - is this OPP enabled/disabled by default
> + * @freq:	Frequency in hertz corresponding to this OPP
> + * @u_volt:	Nominal voltage in microvolts corresponding to this OPP
> + *
> + * OMAP SOCs have a standard set of tuples consisting of frequency and voltage
> + * pairs that the device will support per voltage domain. This is called
> + * Operating Points or OPP. The actual definitions of OMAP Operating Points
> + * varies over silicon within the same family of devices. For a specific
> + * domain, you can have a set of {frequency, voltage} pairs and this is denoted
> + * by an array of omap_opp_def. As the kernel boots and more information is
> + * available, a set of these are activated based on the precise nature of
> + * device the kernel boots up on. It is interesting to remember that each IP
> + * which belongs to a voltage domain may define their own set of OPPs on top
> + * of this - but this is handled by the appropriate driver.
> + */
> +struct omap_opp_def {
> +	bool enabled;
> +	unsigned long freq;
> +	u32 u_volt;
> +};

See above comment on 'struct omap_opp'.  I think these two should be
combined.

I think the initial intent of having them separated so that the
internal struct of 'struct omap_opp' could eventually move to the C
file was the original intent, but I think it aids readability to just
have a single OPP struct.

> +/* Initialization wrapper */
> +#define OMAP_OPP_DEF(_enabled, _freq, _uv)	\
> +{						\
> +	.enabled	= _enabled,		\
> +	.freq		= _freq,		\
> +	.u_volt		= _uv,			\
> +}

nice

> +/* Terminator for the initialization list */
> +#define OMAP_OPP_DEF_TERMINATOR OMAP_OPP_DEF(0, 0, 0)

I'd just drop this and use OMAP_OPP_DEF(0, 0, 0) directly in 
the table.

> +/**
> + * opp_init_list() - Initialize an opp list from the opp definitions
> + * @opp_defs:	Initial opp definitions to create the list.
> + *
> + * This function creates a list of opp definitions and returns a handle.
> + * This list can be used to further validation/search/modifications. New
> + * opp entries can be added to this list by using opp_add().
> + *
> + * In the case of error, ERR_PTR is returned to the caller and should be
> + * appropriately handled with IS_ERR.
> + */
> +struct omap_opp __init *opp_init_list(const struct omap_opp_def *opp_defs);

My original suggestion was that opp_init_list() simply creates a new
but empty list.  Adding OPPs should be done using opp_add().

I guess I'm OK with having the 'bulk add' feature of init_list() but
would rather see a single way to add OPPs.

> +/**
> + * opp_add()  - Add an OPP table from a table definitions
> + * @oppl:	List to add the OPP to
> + * @opp_def:	omap_opp_def to describe the OPP which we want to add to list.
> + *
> + * This function adds an opp definition to the opp list and returns
> + * a handle representing the new OPP list. This handle is then used for further
> + * validation, search, modification operations on the OPP list.
> + *
> + * This function returns the pointer to the allocated list through oppl if
> + * success, else corresponding ERR_PTR value. Caller should NOT free the oppl.
> + * opps_defs can be freed after use.
> + *
> + * NOTE: caller should assume that on success, oppl is probably populated with
> + * a new handle and the new handle should be used for further referencing
> + */
> +struct omap_opp *opp_add(struct omap_opp *oppl,
> +			 const struct omap_opp_def *opp_def);

c.f. proposal to drop omap_opp_def.

otherwise, ack.

> +/**
> + * opp_enable() - Enable a specific OPP
> + * @opp:	Pointer to opp
> + *
> + * Enables a provided opp. If the operation is valid, this returns 0, else the
> + * corresponding error value.
> + *
> + * OPP used here is from the the opp_is_valid/opp_has_freq or other search
> + * functions
> + */
> +int opp_enable(struct omap_opp *opp);

ack

> +/**
> + * opp_disable() - Disable a specific OPP
> + * @opp:	Pointer to opp
> + *axs
> + * Disables a provided opp. If the operation is valid, this returns 0, else the
> + * corresponding error value.
> + *
> + * OPP used here is from the the opp_is_valid/opp_has_freq or other search
> + * functions
> + */
> +int opp_disable(struct omap_opp *opp);

ack

Kevin

> +#endif		/* __ASM_ARM_OMAP_OPP_H */
> diff --git a/arch/arm/plat-omap/opp.c b/arch/arm/plat-omap/opp.c
> new file mode 100644
> index 0000000..c4dc07b
> --- /dev/null
> +++ b/arch/arm/plat-omap/opp.c
> @@ -0,0 +1,271 @@
> +/*
> + * OMAP OPP Interface
> + *
> + * Copyright (C) 2009 Texas Instruments Incorporated.
> + *	Nishanth Menon
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/errno.h>
> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +
> +#include <plat/opp.h>
> +
> +/*
> + * DEPRECATED: Meant to detect end of opp array
> + * This is meant to help co-exist with current SRF etc
> + * TODO: REMOVE!
> + */
> +#define OPP_TERM(opp) (!(opp)->rate && !(opp)->vsel && !(opp)->enabled)
> +
> +/*
> + * DEPRECATED: Meant to convert vsel value to uVolt
> + * This is meant to help co-exist with current SRF etc
> + * TODO: REMOVE!
> + */
> +static inline unsigned long vsel_to_uv(const u8 vsel)
> +{
> +	return (((vsel * 125) + 6000)) * 100;
> +}
> +
> +/*
> + * DEPRECATED: Meant to convert uVolt to vsel value
> + * This is meant to help co-exist with current SRF etc
> + * TODO: REMOVE!
> + */
> +static inline unsigned char uv_to_vsel(unsigned long uV)
> +{
> +	return ((uV / 100) - 6000) / 125;
> +}
> +
> +unsigned long opp_get_voltage(const struct omap_opp *opp)
> +{
> +	if (unlikely(!opp || IS_ERR(opp)) || !opp->enabled) {
> +		pr_err("%s: Invalid parameters being passed\n", __func__);
> +		return 0;
> +	}
> +	return vsel_to_uv(opp->vsel);
> +}
> +
> +unsigned long opp_get_freq(const struct omap_opp *opp)
> +{
> +	if (unlikely(!opp || IS_ERR(opp)) || !opp->enabled) {
> +		pr_err("%s: Invalid parameters being passed\n", __func__);
> +		return 0;
> +	}
> +	return opp->rate;
> +}
> +
> +int opp_get_opp_count(const struct omap_opp *oppl)
> +{
> +	struct omap_opp *opp;
> +	u8 n = 0;
> +
> +	if (unlikely(!oppl || IS_ERR(oppl))) {
> +		pr_err("%s: Invalid parameters being passed\n", __func__);
> +		return -EINVAL;
> +	}
> +	opp = (struct omap_opp *)oppl;
> +	opp++;			/* skip initial terminator */
> +	while (!OPP_TERM(opp)) {
> +		if (opp->enabled)
> +			n++;
> +		opp++;
> +	}
> +	return n;
> +}
> +
> +struct omap_opp *opp_find_freq_exact(struct omap_opp *oppl,
> +				     unsigned long freq, bool enabled)
> +{
> +	struct omap_opp *opp = (struct omap_opp *)oppl;
> +
> +	if (unlikely(!oppl || IS_ERR(oppl))) {
> +		pr_err("%s: Invalid parameters being passed\n", __func__);
> +		return ERR_PTR(-EINVAL);
> +	}
> +	/* skip initial terminator */
> +	if (OPP_TERM(opp))
> +		opp++;
> +	while (!OPP_TERM(opp)) {
> +		if ((opp->rate == freq) && (opp->enabled == enabled))
> +			break;
> +		opp++;
> +	}
> +
> +	return OPP_TERM(opp) ? ERR_PTR(-ENOENT) : opp;
> +}
> +
> +struct omap_opp *opp_find_freq_approx(struct omap_opp *oppl,
> +				      unsigned long *freq, u8 dir_flag)
> +{
> +	struct omap_opp *opp = (struct omap_opp *)oppl;
> +
> +	if (unlikely(!oppl || IS_ERR(oppl) || !freq || IS_ERR(freq))) {
> +		pr_err("%s: Invalid parameters being passed\n", __func__);
> +		return ERR_PTR(-EINVAL);
> +	}
> +	/* skip initial terminator */
> +	if (OPP_TERM(opp)) {
> +		opp++;
> +		/* If searching init list for a high val, skip to very top */
> +		if (dir_flag == OPP_SEARCH_LOW)
> +			while (!OPP_TERM(opp + 1))
> +				opp++;
> +	}
> +	while (!OPP_TERM(opp)) {
> +		if (opp->enabled &&
> +		    (((dir_flag == OPP_SEARCH_HIGH) && (opp->rate >= *freq)) ||
> +		     ((dir_flag == OPP_SEARCH_LOW) && (opp->rate <= *freq))))
> +			break;
> +		opp += (dir_flag == OPP_SEARCH_LOW) ? -1 : 1;
> +	}
> +
> +	if (OPP_TERM(opp))
> +		return ERR_PTR(-ENOENT);
> +
> +	*freq = opp->rate;
> +	return opp;
> +}
> +
> +/* wrapper to reuse converting opp_def to opp struct */
> +static void omap_opp_populate(struct omap_opp *opp,
> +			      const struct omap_opp_def *opp_def)
> +{
> +	opp->rate = opp_def->freq;
> +	opp->enabled = opp_def->enabled;
> +	opp->vsel = uv_to_vsel(opp_def->u_volt);
> +	/* round off to higher voltage */
> +	if (opp_def->u_volt > vsel_to_uv(opp->vsel))
> +		opp->vsel++;
> +}
> +
> +struct omap_opp *opp_add(struct omap_opp *oppl,
> +			 const struct omap_opp_def *opp_def)
> +{
> +	struct omap_opp *opp, *oppt, *oppr;
> +	u8 n, i, ins;
> +
> +	if (unlikely(!oppl || IS_ERR(oppl) || !opp_def || IS_ERR(opp_def))) {
> +		pr_err("%s: Invalid params being passed\n", __func__);
> +		return ERR_PTR(-EINVAL);
> +	}
> +	/* need a start terminator.. */
> +	if (unlikely(!OPP_TERM(oppl))) {
> +		pr_err("%s: Expected a start terminator!!\n", __func__);
> +		return ERR_PTR(-EINVAL);
> +	}
> +	n = 0;
> +	opp = oppl;
> +	opp++;
> +	while (!OPP_TERM(opp)) {
> +		n++;
> +		opp++;
> +	}
> +	/* lets now reallocate memory */
> +	oppr = kmalloc(sizeof(struct omap_opp) * (n + 3), GFP_KERNEL);
> +	if (!oppr) {
> +		pr_err("%s: No memory for new opp array\n", __func__);
> +		return ERR_PTR(-ENOMEM);
> +	}
> +
> +	/* Simple insertion sort */
> +	opp = oppl;
> +	oppt = oppr;
> +	ins = 0;
> +	i = 0;
> +	do {
> +		if (ins || opp->rate < opp_def->freq) {
> +			memcpy(oppt, opp, sizeof(struct omap_opp));
> +			opp++;
> +		} else {
> +			omap_opp_populate(oppt, opp_def);
> +			ins++;
> +		}
> +		oppt->opp_id = i;
> +		oppt++;
> +		i++;
> +	} while (!OPP_TERM(opp));
> +
> +	/* If nothing got inserted, this belongs to the end */
> +	if (!ins) {
> +		omap_opp_populate(oppt, opp_def);
> +		oppt->opp_id = i;
> +		oppt++;
> +	}
> +	/* Put the terminator back on */
> +	memcpy(oppt, opp, sizeof(struct omap_opp));
> +
> +	/* Free the old list */
> +	kfree(oppl);
> +
> +	return oppr;
> +}
> +
> +struct omap_opp __init *opp_init_list(const struct omap_opp_def *opp_defs)
> +{
> +	struct omap_opp_def *t = (struct omap_opp_def *)opp_defs;
> +	struct omap_opp *opp, *oppl;
> +	u8 n = 0, i = 1;
> +
> +	if (unlikely(!opp_defs || IS_ERR(opp_defs))) {
> +		pr_err("%s: Invalid params being passed\n", __func__);
> +		return ERR_PTR(-EINVAL);
> +	}
> +	/* Grab a count */
> +	while (t->enabled || t->freq || t->u_volt) {
> +		n++;
> +		t++;
> +	}
> +
> +	oppl = kmalloc(sizeof(struct omap_opp) * (n + 2), GFP_KERNEL);
> +	if (!oppl) {
> +		pr_err("%s: No memory for opp array\n", __func__);
> +		return ERR_PTR(-ENOMEM);
> +	}
> +	opp = oppl;
> +	/* Setup start terminator - SRF depends on this for indexing :( */
> +	opp->rate = 0;
> +	opp->enabled = 0;
> +	opp->vsel = 0;
> +	opp++;
> +	while (n) {
> +		omap_opp_populate(opp, opp_defs);
> +		opp->opp_id = i;
> +		n--;
> +		opp++;
> +		opp_defs++;
> +		i++;
> +	}
> +	/* Setup terminator - this is for our search algos */
> +	opp->rate = 0;
> +	opp->enabled = 0;
> +	opp->vsel = 0;
> +	return oppl;
> +}
> +
> +int opp_enable(struct omap_opp *opp)
> +{
> +	if (unlikely(!opp || IS_ERR(opp))) {
> +		pr_err("%s: Invalid parameters being passed\n", __func__);
> +		return -EINVAL;
> +	}
> +	opp->enabled = true;
> +	return 0;
> +}
> +
> +int opp_disable(struct omap_opp *opp)
> +{
> +	if (unlikely(!opp || IS_ERR(opp))) {
> +		pr_err("%s: Invalid parameters being passed\n", __func__);
> +		return -EINVAL;
> +	}
> +	opp->enabled = false;
> +	return 0;
> +}
> -- 
> 1.6.3.3

Re: [PATCH 02/10 V4] omap3: pm: introduce opp accessor functions

[Nishanth Menon]

Kevin Hilman had written, on 12/10/2009 05:25 PM, the following:
Thanks for the acks..

> Nishanth Menon <nm@ti.com> writes:
[...]

>> diff --git a/arch/arm/plat-omap/include/plat/opp.h b/arch/arm/plat-omap/include/plat/opp.h
>> new file mode 100644
>> index 0000000..341c02b

[...]

>> +/**
>> + * struct omap_opp - OMAP OPP description structure
>> + * @enabled: true/false - marking this OPP as enabled/disabled
>> + * @rate:    Frequency in hertz
>> + * @opp_id:  (DEPRECATED) opp identifier
>> + * @vsel:    Voltage in volt processor level(this usage is
>> + *           DEPRECATED to use Voltage in microvolts in future)
>> + *           uV = ((vsel * 12.5) + 600) * 1000
>> + *
>> + * This structure stores the OPP information for a given domain.
>> + * Due to legacy reasons, this structure is currently exposed and
>> + * will soon be removed elsewhere and will only be used as a handle
>> + * from the OPP internal referencing mechanism
>> + */
>> +struct omap_opp {
>> +     bool enabled;
>> +     unsigned long rate;
>> +     u8 opp_id __deprecated;
>> +     u16 vsel;
> 
> How about we add 'u32 voltage' here and mark vsel as __deprecated.  Then
> we no longer need both an 'struct omap_opp' and a 'struct omap_opp_def'.
> 
> Or even better, with the uv <--> vsel conversion macros you added,
> couldn't we alrady define the OPPs in terms of voltage, and drop the
> vsel already?

we should do that once we fix SR + resource34xx (underworks) - they 
directly use them and I kept my "status quo" rule switch on ;). Once it 
is done, vsel becomes voltage and an unsigned long. and we can manage it 
inside opp.c anyway we choose. For this starting set, I dont think we 
should do this.

[...]
> 
>> +/**
>> + * opp_find_freq_exact() - search for an exact frequency
>> + * @oppl:    OPP list
>> + * @freq:    frequency to search for
>> + * @enabled: enabled/disabled OPP to search for
>> + *
>> + * searches for the match in the opp list and returns handle to the matching
>> + * opp if found, else returns ERR_PTR in case of error and should be handled
>> + * using IS_ERR.
>> + *
>> + * Note enabled is a modifier for the search. if enabled=true, then the match is
>> + * for exact matching frequency and is enabled. if true, the match is for exact
>> + * frequency which is disabled.
>> + */
>> +struct omap_opp *opp_find_freq_exact(struct omap_opp *oppl,
>> +                                  unsigned long freq, bool enabled);
> 
> ack
> 
> I think we could drop the _exact, and just call it opp_find_freq(), but I'm
> ok either way.
shrug.. kinda matches with _approx .. it improves readability esp when 
people look at a usage code 6 months from now and question what 
find_freq is doing and get confused about freq_approx

[...]

>> +/**
>> + * struct omap_opp_def - OMAP OPP Definition
>> + * @enabled: True/false - is this OPP enabled/disabled by default
>> + * @freq:    Frequency in hertz corresponding to this OPP
>> + * @u_volt:  Nominal voltage in microvolts corresponding to this OPP
>> + *
>> + * OMAP SOCs have a standard set of tuples consisting of frequency and voltage
>> + * pairs that the device will support per voltage domain. This is called
>> + * Operating Points or OPP. The actual definitions of OMAP Operating Points
>> + * varies over silicon within the same family of devices. For a specific
>> + * domain, you can have a set of {frequency, voltage} pairs and this is denoted
>> + * by an array of omap_opp_def. As the kernel boots and more information is
>> + * available, a set of these are activated based on the precise nature of
>> + * device the kernel boots up on. It is interesting to remember that each IP
>> + * which belongs to a voltage domain may define their own set of OPPs on top
>> + * of this - but this is handled by the appropriate driver.
>> + */
>> +struct omap_opp_def {
>> +     bool enabled;
>> +     unsigned long freq;
>> +     u32 u_volt;
Comment to self: I should really make the u32 as unsigned long to be in 
sync with what is used elsewhere..(get_voltage)

>> +};
> 
> See above comment on 'struct omap_opp'.  I think these two should be
> combined.
> 
> I think the initial intent of having them separated so that the
> internal struct of 'struct omap_opp' could eventually move to the C
> file was the original intent, but I think it aids readability to just
> have a single OPP struct.

In a few weeks we wont have the struct omap_opp exposed out(once all the 
cleanups are done).. at that point, how would one define an OPP and 
expect to get an handle which they cannot manipulate?

> 
>> +/* Initialization wrapper */
>> +#define OMAP_OPP_DEF(_enabled, _freq, _uv)   \
>> +{                                            \
>> +     .enabled        = _enabled,             \
>> +     .freq           = _freq,                \
>> +     .u_volt         = _uv,                  \
>> +}
> 
> nice
> 
>> +/* Terminator for the initialization list */
>> +#define OMAP_OPP_DEF_TERMINATOR OMAP_OPP_DEF(0, 0, 0)
> 
> I'd just drop this and use OMAP_OPP_DEF(0, 0, 0) directly in
> the table.

Am ok with either (I dont like additional #defs). but terminator helps 
redability a bit though (debatable).. any reasons why u'd like it 0,0,0?
> 
>> +/**
>> + * opp_init_list() - Initialize an opp list from the opp definitions
>> + * @opp_defs:        Initial opp definitions to create the list.
>> + *
>> + * This function creates a list of opp definitions and returns a handle.
>> + * This list can be used to further validation/search/modifications. New
>> + * opp entries can be added to this list by using opp_add().
>> + *
>> + * In the case of error, ERR_PTR is returned to the caller and should be
>> + * appropriately handled with IS_ERR.
>> + */
>> +struct omap_opp __init *opp_init_list(const struct omap_opp_def *opp_defs);
> 
> My original suggestion was that opp_init_list() simply creates a new
> but empty list.  Adding OPPs should be done using opp_add().
> 
> I guess I'm OK with having the 'bulk add' feature of init_list() but
> would rather see a single way to add OPPs.
Reasons why to have a buld add feature in init:
a) There is opp_add below which allows u to add single opp
b) In terms of walk thru code duplication (once this gets used accross 
OMAPs) it is essentially the same thing we do (add each OPP definition 
for a domain)..
c) you dont incur function call latencies. (ok not a big deal.. but still).

> 
>> +/**
>> + * opp_add()  - Add an OPP table from a table definitions
>> + * @oppl:    List to add the OPP to
>> + * @opp_def: omap_opp_def to describe the OPP which we want to add to list.
>> + *
>> + * This function adds an opp definition to the opp list and returns
>> + * a handle representing the new OPP list. This handle is then used for further
>> + * validation, search, modification operations on the OPP list.
>> + *
>> + * This function returns the pointer to the allocated list through oppl if
>> + * success, else corresponding ERR_PTR value. Caller should NOT free the oppl.
>> + * opps_defs can be freed after use.
>> + *
>> + * NOTE: caller should assume that on success, oppl is probably populated with
>> + * a new handle and the new handle should be used for further referencing
>> + */
>> +struct omap_opp *opp_add(struct omap_opp *oppl,
>> +                      const struct omap_opp_def *opp_def);
> 
> c.f. proposal to drop omap_opp_def.
explained above.

> 
> otherwise, ack.
> 

-- 
Regards,
Nishanth Menon

Re: [PATCH 02/10 V4] omap3: pm: introduce opp accessor functions

[Eduardo Valentin]

On Fri, Dec 11, 2009 at 01:41:46AM +0100, ext Nishanth Menon wrote:
> Kevin Hilman had written, on 12/10/2009 05:25 PM, the following:
> Thanks for the acks..
> 
> > Nishanth Menon <nm@ti.com> writes:
> [...]
> 
> >> diff --git a/arch/arm/plat-omap/include/plat/opp.h b/arch/arm/plat-omap/include/plat/opp.h
> >> new file mode 100644
> >> index 0000000..341c02b
> 
> [...]
> 
> >> +/**
> >> + * struct omap_opp - OMAP OPP description structure
> >> + * @enabled: true/false - marking this OPP as enabled/disabled
> >> + * @rate:    Frequency in hertz
> >> + * @opp_id:  (DEPRECATED) opp identifier
> >> + * @vsel:    Voltage in volt processor level(this usage is
> >> + *           DEPRECATED to use Voltage in microvolts in future)
> >> + *           uV = ((vsel * 12.5) + 600) * 1000
> >> + *
> >> + * This structure stores the OPP information for a given domain.
> >> + * Due to legacy reasons, this structure is currently exposed and
> >> + * will soon be removed elsewhere and will only be used as a handle
> >> + * from the OPP internal referencing mechanism
> >> + */
> >> +struct omap_opp {
> >> +     bool enabled;
> >> +     unsigned long rate;
> >> +     u8 opp_id __deprecated;
> >> +     u16 vsel;
> > 
> > How about we add 'u32 voltage' here and mark vsel as __deprecated.  Then
> > we no longer need both an 'struct omap_opp' and a 'struct omap_opp_def'.
> > 
> > Or even better, with the uv <--> vsel conversion macros you added,
> > couldn't we alrady define the OPPs in terms of voltage, and drop the
> > vsel already?
> 
> we should do that once we fix SR + resource34xx (underworks) - they 
> directly use them and I kept my "status quo" rule switch on ;). Once it 
> is done, vsel becomes voltage and an unsigned long. and we can manage it 
> inside opp.c anyway we choose. For this starting set, I dont think we 
> should do this.

I'm OK if you have the plan to do it in steps. But might be useful to have some
REVISIT / TODO comment on top of things you know you are going to change afterwards.

It is not mandatory, but it helps to keep track of what is in your plans.

> 
> [...]
> > 
> >> +/**
> >> + * opp_find_freq_exact() - search for an exact frequency
> >> + * @oppl:    OPP list
> >> + * @freq:    frequency to search for
> >> + * @enabled: enabled/disabled OPP to search for
> >> + *
> >> + * searches for the match in the opp list and returns handle to the matching
> >> + * opp if found, else returns ERR_PTR in case of error and should be handled
> >> + * using IS_ERR.
> >> + *
> >> + * Note enabled is a modifier for the search. if enabled=true, then the match is
> >> + * for exact matching frequency and is enabled. if true, the match is for exact
> >> + * frequency which is disabled.
> >> + */
> >> +struct omap_opp *opp_find_freq_exact(struct omap_opp *oppl,
> >> +                                  unsigned long freq, bool enabled);
> > 
> > ack
> > 
> > I think we could drop the _exact, and just call it opp_find_freq(), but I'm
> > ok either way.
> shrug.. kinda matches with _approx .. it improves readability esp when 
> people look at a usage code 6 months from now and question what 
> find_freq is doing and get confused about freq_approx
> 
> [...]
> 
> >> +/**
> >> + * struct omap_opp_def - OMAP OPP Definition
> >> + * @enabled: True/false - is this OPP enabled/disabled by default
> >> + * @freq:    Frequency in hertz corresponding to this OPP
> >> + * @u_volt:  Nominal voltage in microvolts corresponding to this OPP
> >> + *
> >> + * OMAP SOCs have a standard set of tuples consisting of frequency and voltage
> >> + * pairs that the device will support per voltage domain. This is called
> >> + * Operating Points or OPP. The actual definitions of OMAP Operating Points
> >> + * varies over silicon within the same family of devices. For a specific
> >> + * domain, you can have a set of {frequency, voltage} pairs and this is denoted
> >> + * by an array of omap_opp_def. As the kernel boots and more information is
> >> + * available, a set of these are activated based on the precise nature of
> >> + * device the kernel boots up on. It is interesting to remember that each IP
> >> + * which belongs to a voltage domain may define their own set of OPPs on top
> >> + * of this - but this is handled by the appropriate driver.
> >> + */
> >> +struct omap_opp_def {
> >> +     bool enabled;
> >> +     unsigned long freq;
> >> +     u32 u_volt;
> Comment to self: I should really make the u32 as unsigned long to be in 
> sync with what is used elsewhere..(get_voltage)
> 
> >> +};
> > 
> > See above comment on 'struct omap_opp'.  I think these two should be
> > combined.
> > 
> > I think the initial intent of having them separated so that the
> > internal struct of 'struct omap_opp' could eventually move to the C
> > file was the original intent, but I think it aids readability to just
> > have a single OPP struct.
> 
> In a few weeks we wont have the struct omap_opp exposed out(once all the 
> cleanups are done).. at that point, how would one define an OPP and 
> expect to get an handle which they cannot manipulate?
> 
> > 
> >> +/* Initialization wrapper */
> >> +#define OMAP_OPP_DEF(_enabled, _freq, _uv)   \
> >> +{                                            \
> >> +     .enabled        = _enabled,             \
> >> +     .freq           = _freq,                \
> >> +     .u_volt         = _uv,                  \
> >> +}
> > 
> > nice
> > 
> >> +/* Terminator for the initialization list */
> >> +#define OMAP_OPP_DEF_TERMINATOR OMAP_OPP_DEF(0, 0, 0)
> > 
> > I'd just drop this and use OMAP_OPP_DEF(0, 0, 0) directly in
> > the table.
> 
> Am ok with either (I dont like additional #defs). but terminator helps 
> redability a bit though (debatable).. any reasons why u'd like it 0,0,0?
> > 
> >> +/**
> >> + * opp_init_list() - Initialize an opp list from the opp definitions
> >> + * @opp_defs:        Initial opp definitions to create the list.
> >> + *
> >> + * This function creates a list of opp definitions and returns a handle.
> >> + * This list can be used to further validation/search/modifications. New
> >> + * opp entries can be added to this list by using opp_add().
> >> + *
> >> + * In the case of error, ERR_PTR is returned to the caller and should be
> >> + * appropriately handled with IS_ERR.
> >> + */
> >> +struct omap_opp __init *opp_init_list(const struct omap_opp_def *opp_defs);
> > 
> > My original suggestion was that opp_init_list() simply creates a new
> > but empty list.  Adding OPPs should be done using opp_add().
> > 
> > I guess I'm OK with having the 'bulk add' feature of init_list() but
> > would rather see a single way to add OPPs.
> Reasons why to have a buld add feature in init:
> a) There is opp_add below which allows u to add single opp
> b) In terms of walk thru code duplication (once this gets used accross 
> OMAPs) it is essentially the same thing we do (add each OPP definition 
> for a domain)..
> c) you dont incur function call latencies. (ok not a big deal.. but still).
> 
> > 
> >> +/**
> >> + * opp_add()  - Add an OPP table from a table definitions
> >> + * @oppl:    List to add the OPP to
> >> + * @opp_def: omap_opp_def to describe the OPP which we want to add to list.
> >> + *
> >> + * This function adds an opp definition to the opp list and returns
> >> + * a handle representing the new OPP list. This handle is then used for further
> >> + * validation, search, modification operations on the OPP list.
> >> + *
> >> + * This function returns the pointer to the allocated list through oppl if
> >> + * success, else corresponding ERR_PTR value. Caller should NOT free the oppl.
> >> + * opps_defs can be freed after use.
> >> + *
> >> + * NOTE: caller should assume that on success, oppl is probably populated with
> >> + * a new handle and the new handle should be used for further referencing
> >> + */
> >> +struct omap_opp *opp_add(struct omap_opp *oppl,
> >> +                      const struct omap_opp_def *opp_def);
> > 
> > c.f. proposal to drop omap_opp_def.
> explained above.
> 
> > 
> > otherwise, ack.
> > 
> 
> -- 
> Regards,
> Nishanth Menon

-- 
Eduardo Valentin

Re: [PATCH 09/10 V4] omap3: pm: introduce 3630 opps

[Eduardo Valentin]

Hello Nishanth,

On Wed, Dec 09, 2009 at 07:17:14AM +0100, ext Nishanth Menon wrote:
> Introduce the OMAP3630 OPPs including the defined OPP tuples.
> 
> Further information on OMAP3630 can be found here:
> http://focus.ti.com/general/docs/wtbu/wtbuproductcontent.tsp?templateId=6123&navigationId=12836&contentId=52606
> 
> OMAP36xx family introduces:
> VDD1 with 4 OPPs, of which OPP3 & 4 are available only on devices yet
> to be introduced in 36xx family. Meanwhile, VDD2 has 2 opps.
> 
> Device Support of OPPs->
>            |<-3630-600->| (default)
>            |<-      3630-800    ->| (device: TBD)
>            |<-      3630-1000            ->| (device: TBD)
> H/w OPP-> OPP50 OPP100       OPP-Turbo   OPP1G-SB
> VDD1      OPP1  OPP2         OPP3        OPP4
> VDD2      OPP1  OPP2         OPP2        OPP2
> 
> Note:
> a) TI h/w naming for OPPs are now standardized in terms of OPP50, 100,
>    Turbo and SB. This maps as shown above to the opp IDs (s/w term).
> b) For boards which need custom VDD1/2 OPPs, the opp table can be
>    updated by the board file on a need basis after the
>    omap3_pm_init_opp_table call. The OPPs introduced here are the
>    official OPP table at this point in time.
> 

[ ... ]

>  
> +static struct omap_opp_def __initdata omap36xx_mpu_rate_table[] = {
> +	/* OPP1 - OPP50 */
> +	OMAP_OPP_DEF(true,  300000000, 930000),
> +	/* OPP2 - OPP100 */
> +	OMAP_OPP_DEF(true,  600000000, 1100000),
> +	/* OPP3 - OPP-Turbo */
> +	OMAP_OPP_DEF(false, 800000000, 1260000),
> +	/* OPP4 - OPP-SB */
> +	OMAP_OPP_DEF(false, 1000000000, 1310000),
> +	OMAP_OPP_DEF_TERMINATOR

What else needs to be done in order to enable OPP-Turbo & OPP-SB ?


> +};
> +
> +static struct omap_opp_def __initdata omap36xx_l3_rate_table[] = {
> +	/* OPP1 - OPP50 */
> +	OMAP_OPP_DEF(true, 100000000, 930000),
> +	/* OPP2 - OPP100, OPP-Turbo, OPP-SB */
> +	OMAP_OPP_DEF(true, 200000000, 1137500),
> +	OMAP_OPP_DEF_TERMINATOR
> +};
> +
> +static struct omap_opp_def __initdata omap36xx_dsp_rate_table[] = {
> +	/* OPP1 - OPP50 */
> +	OMAP_OPP_DEF(true,  260000000, 930000),
> +	/* OPP2 - OPP100 */
> +	OMAP_OPP_DEF(true,  520000000, 1100000),
> +	/* OPP3 - OPP-Turbo */
> +	OMAP_OPP_DEF(false, 660000000, 1260000),
> +	/* OPP4 - OPP-SB */
> +	OMAP_OPP_DEF(false, 875000000, 1310000),
> +	OMAP_OPP_DEF_TERMINATOR
> +};
> +
>  /* OMAP3 Rate Table */
>  struct omap_opp *omap3_mpu_rate_table;
>  struct omap_opp *omap3_dsp_rate_table;
> @@ -1296,18 +1328,27 @@ static void __init configure_vc(void)
>  void __init omap3_pm_init_opp_table(void)
>  {
>  	int i;
> +	struct omap_opp_def **omap3_opp_def_list;
>  	struct omap_opp_def *omap34xx_opp_def_list[] = {
>  		omap34xx_mpu_rate_table,
>  		omap34xx_l3_rate_table,
>  		omap34xx_dsp_rate_table
>  	};
> +	struct omap_opp_def *omap36xx_opp_def_list[] = {
> +		omap36xx_mpu_rate_table,
> +		omap36xx_l3_rate_table,
> +		omap36xx_dsp_rate_table
> +	};
>  	struct omap_opp **omap3_rate_tables[] = {
>  		&omap3_mpu_rate_table,
>  		&omap3_l3_rate_table,
>  		&omap3_dsp_rate_table
>  	};
> +
> +	omap3_opp_def_list = cpu_is_omap3630() ? omap36xx_opp_def_list :
> +				omap34xx_opp_def_list;
>  	for (i = 0; i < ARRAY_SIZE(omap3_rate_tables); i++) {
> -		*omap3_rate_tables[i] = opp_init_list(omap34xx_opp_def_list[i]);
> +		*omap3_rate_tables[i] = opp_init_list(omap3_opp_def_list[i]);
>  		/* We dont want half configured system at the moment */
>  		BUG_ON(IS_ERR(omap3_rate_tables[i]));
>  	}
> -- 
> 1.6.3.3

-- 
Eduardo Valentin

Re: [PATCH 03/10 V4] omap3: pm: use opp accessor functions for omap34xx

[Eduardo Valentin]

Hello Nishanth,

You might have missed last tero's comment. But I'm adding those again here.

On Wed, Dec 09, 2009 at 07:17:08AM +0100, ext Nishanth Menon wrote:
> Move the definitions from omap3-opp.h to pm34xx.c. The definitions
> are now based on omap_opp_def instead of omap_opp itself.
> Since the opp.h has the omap_opp definition, omap-pm.h conflicts and
> has been removed in favor of opp.h.
> 
> omap3_pm_init_opp_table is used to initialize the OPP table and
> relevant board files which have omap2_init_common_hw called with opp
> arrays have been updated with omap3_pm_init_opp_table.
> 
> This change now allows us to dynamically register OPPs to the system
> based on silicon type we detect.
> 
> NOTE: This introduces the following warnings highlighting areas we
> need to cleanup:
> arch/arm/mach-omap2/smartreflex.c: In function 'get_opp':
> arch/arm/mach-omap2/smartreflex.c:161: warning: 'opp_id' is deprecated (declared at arch/arm/plat-omap/include/plat/opp.h:33)
> arch/arm/mach-omap2/smartreflex.c:164: warning: 'opp_id' is deprecated (declared at arch/arm/plat-omap/include/plat/opp.h:33)
> arch/arm/mach-omap2/smartreflex.c:166: warning: 'opp_id' is deprecated (declared at arch/arm/plat-omap/include/plat/opp.h:33)
> arch/arm/mach-omap2/smartreflex.c:168: warning: 'opp_id' is deprecated (declared at arch/arm/plat-omap/include/plat/opp.h:33)
> arch/arm/mach-omap2/resource34xx.c: In function 'get_opp':
> arch/arm/mach-omap2/resource34xx.c:165: warning: 'opp_id' is deprecated (declared at arch/arm/plat-omap/include/plat/opp.h:33)
> arch/arm/mach-omap2/resource34xx.c:168: warning: 'opp_id' is deprecated (declared at arch/arm/plat-omap/include/plat/opp.h:33)
> arch/arm/mach-omap2/resource34xx.c:170: warning: 'opp_id' is deprecated (declared at arch/arm/plat-omap/include/plat/opp.h:33)
> arch/arm/mach-omap2/resource34xx.c:172: warning: 'opp_id' is deprecated (declared at arch/arm/plat-omap/include/plat/opp.h:33)
> arch/arm/mach-omap2/resource34xx.c: In function 'program_opp':
> arch/arm/mach-omap2/resource34xx.c:284: warning: 'opp_id' is deprecated (declared at arch/arm/plat-omap/include/plat/opp.h:33)
> arch/arm/mach-omap2/resource34xx.c:285: warning: 'opp_id' is deprecated (declared at arch/arm/plat-omap/include/plat/opp.h:33)
> 
> Cc: Benoit Cousson <b-cousson@ti.com>
> Cc: Eduardo Valentin <eduardo.valentin@nokia.com>
> Cc: Kevin Hilman <khilman@deeprootsystems.com>
> Cc: Madhusudhan Chikkature Rajashekar <madhu.cr@ti.com>
> Cc: Paul Walmsley <paul@pwsan.com>
> Cc: Romit Dasgupta <romit@ti.com>
> Cc: Sanjeev Premi <premi@ti.com>
> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Cc: Sergio Alberto Aguirre Rodriguez <saaguirre@ti.com>
> Cc: Tero Kristo <Tero.Kristo@nokia.com>
> Cc: Thara Gopinath <thara@ti.com>
> Cc: Vishwanath Sripathy <vishwanath.bs@ti.com>
> 
> Signed-off-by: Nishanth Menon <nm@ti.com>
> ---
>  arch/arm/mach-omap2/board-3430sdp.c       |    1 +
>  arch/arm/mach-omap2/board-omap3beagle.c   |    1 +
>  arch/arm/mach-omap2/board-omap3evm.c      |    1 +
>  arch/arm/mach-omap2/board-rx51.c          |    1 +
>  arch/arm/mach-omap2/board-zoom2.c         |    2 +
>  arch/arm/mach-omap2/omap3-opp.h           |   58 +------------------------
>  arch/arm/mach-omap2/pm.h                  |    6 +++
>  arch/arm/mach-omap2/pm34xx.c              |   65 +++++++++++++++++++++++++++++
>  arch/arm/plat-omap/include/plat/omap-pm.h |   17 +-------
>  9 files changed, 81 insertions(+), 71 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/board-3430sdp.c b/arch/arm/mach-omap2/board-3430sdp.c
> index eac529f..0ec8327 100644
> --- a/arch/arm/mach-omap2/board-3430sdp.c
> +++ b/arch/arm/mach-omap2/board-3430sdp.c
> @@ -220,6 +220,7 @@ static void __init omap_3430sdp_init_irq(void)
>  {
>         omap_board_config = sdp3430_config;
>         omap_board_config_size = ARRAY_SIZE(sdp3430_config);
> +       omap3_pm_init_opp_table();
>         omap3_pm_init_vc(&omap3_setuptime_table);
>         omap3_pm_init_cpuidle(omap3_cpuidle_params_table);
>         omap2_init_common_hw(hyb18m512160af6_sdrc_params, NULL, omap3_mpu_rate_table,
> diff --git a/arch/arm/mach-omap2/board-omap3beagle.c b/arch/arm/mach-omap2/board-omap3beagle.c
> index 2ec3520..a937238 100644
> --- a/arch/arm/mach-omap2/board-omap3beagle.c
> +++ b/arch/arm/mach-omap2/board-omap3beagle.c
> @@ -361,6 +361,7 @@ static void __init omap3_beagle_init_irq(void)
>  {
>         omap_board_config = omap3_beagle_config;
>         omap_board_config_size = ARRAY_SIZE(omap3_beagle_config);
> +       omap3_pm_init_opp_table();
>         omap2_init_common_hw(mt46h32m32lf6_sdrc_params,
>                              mt46h32m32lf6_sdrc_params, omap3_mpu_rate_table,
>                              omap3_dsp_rate_table, omap3_l3_rate_table);
> diff --git a/arch/arm/mach-omap2/board-omap3evm.c b/arch/arm/mach-omap2/board-omap3evm.c
> index 8130eca..44a5861 100644
> --- a/arch/arm/mach-omap2/board-omap3evm.c
> +++ b/arch/arm/mach-omap2/board-omap3evm.c
> @@ -404,6 +404,7 @@ static void __init omap3_evm_init_irq(void)
>  {
>         omap_board_config = omap3_evm_config;
>         omap_board_config_size = ARRAY_SIZE(omap3_evm_config);
> +       omap3_pm_init_opp_table();
>         omap2_init_common_hw(mt46h32m32lf6_sdrc_params, NULL, omap3_mpu_rate_table,
>                              omap3_dsp_rate_table, omap3_l3_rate_table);
>         omap_init_irq();
> diff --git a/arch/arm/mach-omap2/board-rx51.c b/arch/arm/mach-omap2/board-rx51.c
> index 2f1c2be..997fd1c 100644
> --- a/arch/arm/mach-omap2/board-rx51.c
> +++ b/arch/arm/mach-omap2/board-rx51.c
> @@ -103,6 +103,7 @@ static void __init rx51_init_irq(void)
> 
>         omap_board_config = rx51_config;
>         omap_board_config_size = ARRAY_SIZE(rx51_config);
> +       omap3_pm_init_opp_table();
>         omap3_pm_init_cpuidle(rx51_cpuidle_params);
>         sdrc_params = rx51_get_sdram_timings();
>         omap2_init_common_hw(sdrc_params, sdrc_params,


Add the following in order to do not break compilation:
diff --git a/arch/arm/mach-omap2/board-rx51.c b/arch/arm/mach-omap2/board-rx51.c
index 527f0c6..1f24933 100644
--- a/arch/arm/mach-omap2/board-rx51.c
+++ b/arch/arm/mach-omap2/board-rx51.c
@@ -31,6 +31,7 @@
 #include <plat/gpmc.h>
 #include <plat/usb.h>
 
+#include "pm.h"
 #include "omap3-opp.h"
 
 #define RX51_GPIO_SLEEP_IND 162


> diff --git a/arch/arm/mach-omap2/board-zoom2.c b/arch/arm/mach-omap2/board-zoom2.c
> index dcc5fb8..9d5b078 100644
> --- a/arch/arm/mach-omap2/board-zoom2.c
> +++ b/arch/arm/mach-omap2/board-zoom2.c
> @@ -24,10 +24,12 @@
>  #include <mach/board-zoom.h>
> 
>  #include "sdram-micron-mt46h32m32lf-6.h"
> +#include "pm.h"
>  #include "omap3-opp.h"
> 
>  static void __init omap_zoom2_init_irq(void)
>  {
> +       omap3_pm_init_opp_table();
>         omap2_init_common_hw(mt46h32m32lf6_sdrc_params,
>                                  mt46h32m32lf6_sdrc_params, omap3_mpu_rate_table,
>                                  omap3_dsp_rate_table, omap3_l3_rate_table);
> diff --git a/arch/arm/mach-omap2/omap3-opp.h b/arch/arm/mach-omap2/omap3-opp.h
> index 42557e1..994d8d4 100644
> --- a/arch/arm/mach-omap2/omap3-opp.h
> +++ b/arch/arm/mach-omap2/omap3-opp.h
> @@ -3,60 +3,8 @@
> 
>  #include <plat/omap-pm.h>
> 
> -/* MPU speeds */
> -#define S600M   600000000
> -#define S550M   550000000
> -#define S500M   500000000
> -#define S250M   250000000
> -#define S125M   125000000
> -
> -/* DSP speeds */
> -#define S430M   430000000
> -#define S400M   400000000
> -#define S360M   360000000
> -#define S180M   180000000
> -#define S90M    90000000
> -
> -/* L3 speeds */
> -#define S83M    83000000
> -#define S166M   166000000
> -
> -static struct omap_opp omap3_mpu_rate_table[] = {
> -       {0, 0, 0, 0},
> -       /*OPP1*/
> -       {true, S125M, VDD1_OPP1, 0x1E},
> -       /*OPP2*/
> -       {true, S250M, VDD1_OPP2, 0x26},
> -       /*OPP3*/
> -       {true, S500M, VDD1_OPP3, 0x30},
> -       /*OPP4*/
> -       {true, S550M, VDD1_OPP4, 0x36},
> -       /*OPP5*/
> -       {true, S600M, VDD1_OPP5, 0x3C},
> -};
> -
> -static struct omap_opp omap3_l3_rate_table[] = {
> -       {0, 0, 0, 0},
> -       /*OPP1*/
> -       {false, 0, VDD2_OPP1, 0x1E},
> -       /*OPP2*/
> -       {true, S83M, VDD2_OPP2, 0x24},
> -       /*OPP3*/
> -       {true, S166M, VDD2_OPP3, 0x2C},
> -};
> -
> -static struct omap_opp omap3_dsp_rate_table[] = {
> -       {0, 0, 0, 0},
> -       /*OPP1*/
> -       {true, S90M, VDD1_OPP1, 0x1E},
> -       /*OPP2*/
> -       {true, S180M, VDD1_OPP2, 0x26},
> -       /*OPP3*/
> -       {true, S360M, VDD1_OPP3, 0x30},
> -       /*OPP4*/
> -       {true, S400M, VDD1_OPP4, 0x36},
> -       /*OPP5*/
> -       {true, S430M, VDD1_OPP5, 0x3C},
> -};
> +extern struct omap_opp *omap3_mpu_rate_table;
> +extern struct omap_opp *omap3_dsp_rate_table;
> +extern struct omap_opp *omap3_l3_rate_table;
> 
>  #endif
> diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
> index 7bc86b6..80a1c1d 100644
> --- a/arch/arm/mach-omap2/pm.h
> +++ b/arch/arm/mach-omap2/pm.h
> @@ -58,6 +58,12 @@ static inline void omap3_pm_init_cpuidle(
>  {
>  }
>  #endif
> +/**
> + * omap3_pm_init_opp_table - OMAP opp table lookup called after cpu is detected.
> + * Initialize the basic opp table here, board files could choose to modify opp
> + * table after the basic initialization
> + */
> +extern void omap3_pm_init_opp_table(void);
> 
>  extern int resource_set_opp_level(int res, u32 target_level, int flags);
>  extern int resource_access_opp_lock(int res, int delta);
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index 627a509..e40a036 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -40,6 +40,7 @@
>  #include <plat/dmtimer.h>
>  #include <plat/usb.h>
> 
> +#include <plat/opp.h>
>  #include <plat/resource.h>
> 
>  #include <asm/tlbflush.h>
> @@ -52,6 +53,7 @@
>  #include "prm.h"
>  #include "pm.h"
>  #include "sdrc.h"
> +#include "omap3-opp.h"
> 
>  static int regset_save_on_suspend;
> 
> @@ -100,6 +102,49 @@ static struct prm_setup_vc prm_setup = {
>         .vdd1_off = 0x00,       /* 0.6v */
>  };
> 
> +static struct omap_opp_def __initdata omap34xx_mpu_rate_table[] = {
> +       /* OPP1 */
> +       OMAP_OPP_DEF(true, 125000000, 975000),
> +       /* OPP2 */
> +       OMAP_OPP_DEF(true, 250000000, 1075000),
> +       /* OPP3 */
> +       OMAP_OPP_DEF(true, 500000000, 1200000),
> +       /* OPP4 */
> +       OMAP_OPP_DEF(true, 550000000, 1270000),
> +       /* OPP5 */
> +       OMAP_OPP_DEF(true, 600000000, 1350000),
> +       OMAP_OPP_DEF_TERMINATOR
> +};
> +
> +static struct omap_opp_def __initdata omap34xx_l3_rate_table[] = {
> +       /* OPP1 */
> +       OMAP_OPP_DEF(false, 0, 975000),
> +       /* OPP2 */
> +       OMAP_OPP_DEF(true, 83000000, 1050000),
> +       /* OPP3 */
> +       OMAP_OPP_DEF(true, 166000000, 1150000),
> +       OMAP_OPP_DEF_TERMINATOR
> +};
> +
> +static struct omap_opp_def __initdata omap34xx_dsp_rate_table[] = {
> +       /* OPP1 */
> +       OMAP_OPP_DEF(true, 90000000, 975000),
> +       /* OPP2 */
> +       OMAP_OPP_DEF(true, 180000000, 1075000),
> +       /* OPP3 */
> +       OMAP_OPP_DEF(true, 360000000, 1200000),
> +       /* OPP4 */
> +       OMAP_OPP_DEF(true, 400000000, 1270000),
> +       /* OPP5 */
> +       OMAP_OPP_DEF(true, 430000000, 1350000),
> +       OMAP_OPP_DEF_TERMINATOR
> +};
> +
> +/* OMAP3 Rate Table */
> +struct omap_opp *omap3_mpu_rate_table;
> +struct omap_opp *omap3_dsp_rate_table;
> +struct omap_opp *omap3_l3_rate_table;
> +
>  static inline void omap3_per_save_context(void)
>  {
>         omap_gpio_save_context();
> @@ -1248,6 +1293,26 @@ static void __init configure_vc(void)
>         pm_dbg_regset_init(2);
>  }
> 
> +void __init omap3_pm_init_opp_table(void)
> +{
> +       int i;
> +       struct omap_opp_def *omap34xx_opp_def_list[] = {
> +               omap34xx_mpu_rate_table,
> +               omap34xx_l3_rate_table,
> +               omap34xx_dsp_rate_table
> +       };
> +       struct omap_opp **omap3_rate_tables[] = {
> +               &omap3_mpu_rate_table,
> +               &omap3_l3_rate_table,
> +               &omap3_dsp_rate_table
> +       };
> +       for (i = 0; i < ARRAY_SIZE(omap3_rate_tables); i++) {
> +               *omap3_rate_tables[i] = opp_init_list(omap34xx_opp_def_list[i]);
> +               /* We dont want half configured system at the moment */
> +               BUG_ON(IS_ERR(omap3_rate_tables[i]));
> +       }
> +}
> +
>  static int __init omap3_pm_early_init(void)
>  {
>         prm_clear_mod_reg_bits(OMAP3430_OFFMODE_POL, OMAP3430_GR_MOD,
> diff --git a/arch/arm/plat-omap/include/plat/omap-pm.h b/arch/arm/plat-omap/include/plat/omap-pm.h
> index 5dc2048..aa36339 100644
> --- a/arch/arm/plat-omap/include/plat/omap-pm.h
> +++ b/arch/arm/plat-omap/include/plat/omap-pm.h
> @@ -18,22 +18,7 @@
>  #include <linux/cpufreq.h>
> 
>  #include "powerdomain.h"
> -
> -/**
> - * struct omap_opp - clock frequency-to-OPP ID table for DSP, MPU
> - * @enabled: enabled if true, disabled if false
> - * @rate: target clock rate
> - * @opp_id: OPP ID
> - * @min_vdd: minimum VDD1 voltage (in millivolts) for this OPP
> - *
> - * Operating performance point data.  Can vary by OMAP chip and board.
> - */
> -struct omap_opp {
> -       bool enabled;
> -       unsigned long rate;
> -       u8 opp_id;
> -       u16 vsel;
> -};
> +#include <plat/opp.h>
> 
>  extern struct omap_opp *mpu_opps;
>  extern struct omap_opp *dsp_opps;
> --
> 1.6.3.3

-- 
Eduardo Valentin

RE: [PATCH 03/10 V4] omap3: pm: use opp accessor functions for omap34xx

[Menon, Nishanth]

> From: Eduardo Valentin [mailto:eduardo.valentin@nokia.com]
> Sent: Friday, December 11, 2009 4:29 AM
>
> Hello Nishanth,
>
> You might have missed last tero's comment. But I'm adding those again here.
>
> On Wed, Dec 09, 2009 at 07:17:08AM +0100, ext Nishanth Menon wrote:
> > Move the definitions from omap3-opp.h to pm34xx.c. The definitions
> > are now based on omap_opp_def instead of omap_opp itself.
> > Since the opp.h has the omap_opp definition, omap-pm.h conflicts and
> > has been removed in favor of opp.h.
> >
> > omap3_pm_init_opp_table is used to initialize the OPP table and
> > relevant board files which have omap2_init_common_hw called with opp
> > arrays have been updated with omap3_pm_init_opp_table.
> >
> > This change now allows us to dynamically register OPPs to the system
> > based on silicon type we detect.
> >
> > NOTE: This introduces the following warnings highlighting areas we
> > need to cleanup:
> > arch/arm/mach-omap2/smartreflex.c: In function 'get_opp':
> > arch/arm/mach-omap2/smartreflex.c:161: warning: 'opp_id' is deprecated
> (declared at arch/arm/plat-omap/include/plat/opp.h:33)
> > arch/arm/mach-omap2/smartreflex.c:164: warning: 'opp_id' is deprecated
> (declared at arch/arm/plat-omap/include/plat/opp.h:33)
> > arch/arm/mach-omap2/smartreflex.c:166: warning: 'opp_id' is deprecated
> (declared at arch/arm/plat-omap/include/plat/opp.h:33)
> > arch/arm/mach-omap2/smartreflex.c:168: warning: 'opp_id' is deprecated
> (declared at arch/arm/plat-omap/include/plat/opp.h:33)
> > arch/arm/mach-omap2/resource34xx.c: In function 'get_opp':
> > arch/arm/mach-omap2/resource34xx.c:165: warning: 'opp_id' is deprecated
> (declared at arch/arm/plat-omap/include/plat/opp.h:33)
> > arch/arm/mach-omap2/resource34xx.c:168: warning: 'opp_id' is deprecated
> (declared at arch/arm/plat-omap/include/plat/opp.h:33)
> > arch/arm/mach-omap2/resource34xx.c:170: warning: 'opp_id' is deprecated
> (declared at arch/arm/plat-omap/include/plat/opp.h:33)
> > arch/arm/mach-omap2/resource34xx.c:172: warning: 'opp_id' is deprecated
> (declared at arch/arm/plat-omap/include/plat/opp.h:33)
> > arch/arm/mach-omap2/resource34xx.c: In function 'program_opp':
> > arch/arm/mach-omap2/resource34xx.c:284: warning: 'opp_id' is deprecated
> (declared at arch/arm/plat-omap/include/plat/opp.h:33)
> > arch/arm/mach-omap2/resource34xx.c:285: warning: 'opp_id' is deprecated
> (declared at arch/arm/plat-omap/include/plat/opp.h:33)
> >
> > Cc: Benoit Cousson <b-cousson@ti.com>
> > Cc: Eduardo Valentin <eduardo.valentin@nokia.com>
> > Cc: Kevin Hilman <khilman@deeprootsystems.com>
> > Cc: Madhusudhan Chikkature Rajashekar <madhu.cr@ti.com>
> > Cc: Paul Walmsley <paul@pwsan.com>
> > Cc: Romit Dasgupta <romit@ti.com>
> > Cc: Sanjeev Premi <premi@ti.com>
> > Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> > Cc: Sergio Alberto Aguirre Rodriguez <saaguirre@ti.com>
> > Cc: Tero Kristo <Tero.Kristo@nokia.com>
> > Cc: Thara Gopinath <thara@ti.com>
> > Cc: Vishwanath Sripathy <vishwanath.bs@ti.com>
> >
> > Signed-off-by: Nishanth Menon <nm@ti.com>
> > ---
> >  arch/arm/mach-omap2/board-3430sdp.c       |    1 +
> >  arch/arm/mach-omap2/board-omap3beagle.c   |    1 +
> >  arch/arm/mach-omap2/board-omap3evm.c      |    1 +
> >  arch/arm/mach-omap2/board-rx51.c          |    1 +
> >  arch/arm/mach-omap2/board-zoom2.c         |    2 +
> >  arch/arm/mach-omap2/omap3-opp.h           |   58 +---------------------
> ---
> >  arch/arm/mach-omap2/pm.h                  |    6 +++
> >  arch/arm/mach-omap2/pm34xx.c              |   65
> +++++++++++++++++++++++++++++
> >  arch/arm/plat-omap/include/plat/omap-pm.h |   17 +-------
> >  9 files changed, 81 insertions(+), 71 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/board-3430sdp.c b/arch/arm/mach-
> omap2/board-3430sdp.c
> > index eac529f..0ec8327 100644
> > --- a/arch/arm/mach-omap2/board-3430sdp.c
> > +++ b/arch/arm/mach-omap2/board-3430sdp.c
> > @@ -220,6 +220,7 @@ static void __init omap_3430sdp_init_irq(void)
> >  {
> >         omap_board_config = sdp3430_config;
> >         omap_board_config_size = ARRAY_SIZE(sdp3430_config);
> > +       omap3_pm_init_opp_table();
> >         omap3_pm_init_vc(&omap3_setuptime_table);
> >         omap3_pm_init_cpuidle(omap3_cpuidle_params_table);
> >         omap2_init_common_hw(hyb18m512160af6_sdrc_params, NULL,
> omap3_mpu_rate_table,
> > diff --git a/arch/arm/mach-omap2/board-omap3beagle.c b/arch/arm/mach-
> omap2/board-omap3beagle.c
> > index 2ec3520..a937238 100644
> > --- a/arch/arm/mach-omap2/board-omap3beagle.c
> > +++ b/arch/arm/mach-omap2/board-omap3beagle.c
> > @@ -361,6 +361,7 @@ static void __init omap3_beagle_init_irq(void)
> >  {
> >         omap_board_config = omap3_beagle_config;
> >         omap_board_config_size = ARRAY_SIZE(omap3_beagle_config);
> > +       omap3_pm_init_opp_table();
> >         omap2_init_common_hw(mt46h32m32lf6_sdrc_params,
> >                              mt46h32m32lf6_sdrc_params,
> omap3_mpu_rate_table,
> >                              omap3_dsp_rate_table, omap3_l3_rate_table);
> > diff --git a/arch/arm/mach-omap2/board-omap3evm.c b/arch/arm/mach-
> omap2/board-omap3evm.c
> > index 8130eca..44a5861 100644
> > --- a/arch/arm/mach-omap2/board-omap3evm.c
> > +++ b/arch/arm/mach-omap2/board-omap3evm.c
> > @@ -404,6 +404,7 @@ static void __init omap3_evm_init_irq(void)
> >  {
> >         omap_board_config = omap3_evm_config;
> >         omap_board_config_size = ARRAY_SIZE(omap3_evm_config);
> > +       omap3_pm_init_opp_table();
> >         omap2_init_common_hw(mt46h32m32lf6_sdrc_params, NULL,
> omap3_mpu_rate_table,
> >                              omap3_dsp_rate_table, omap3_l3_rate_table);
> >         omap_init_irq();
> > diff --git a/arch/arm/mach-omap2/board-rx51.c b/arch/arm/mach-
> omap2/board-rx51.c
> > index 2f1c2be..997fd1c 100644
> > --- a/arch/arm/mach-omap2/board-rx51.c
> > +++ b/arch/arm/mach-omap2/board-rx51.c
> > @@ -103,6 +103,7 @@ static void __init rx51_init_irq(void)
> >
> >         omap_board_config = rx51_config;
> >         omap_board_config_size = ARRAY_SIZE(rx51_config);
> > +       omap3_pm_init_opp_table();
> >         omap3_pm_init_cpuidle(rx51_cpuidle_params);
> >         sdrc_params = rx51_get_sdram_timings();
> >         omap2_init_common_hw(sdrc_params, sdrc_params,
>
>
> Add the following in order to do not break compilation:
> diff --git a/arch/arm/mach-omap2/board-rx51.c b/arch/arm/mach-omap2/board-
> rx51.c
> index 527f0c6..1f24933 100644
> --- a/arch/arm/mach-omap2/board-rx51.c
> +++ b/arch/arm/mach-omap2/board-rx51.c
> @@ -31,6 +31,7 @@
>  #include <plat/gpmc.h>
>  #include <plat/usb.h>
>
> +#include "pm.h"
>  #include "omap3-opp.h"
>
>  #define RX51_GPIO_SLEEP_IND 162

Nope - not needed -> are you in sync with l-o pm branch for this git diff? If not, please do - coz l-o pm already has pm.h included.
http://git.kernel.org/?p=linux/kernel/git/khilman/linux-omap-pm.git;a=blob;f=arch/arm/mach-omap2/board-rx51.c;h=2f1c2be6abed8f11d07fb6880090b7857b819d7b;hb=refs/heads/pm#l35

>
>
> > diff --git a/arch/arm/mach-omap2/board-zoom2.c b/arch/arm/mach-
> omap2/board-zoom2.c
> > index dcc5fb8..9d5b078 100644
> > --- a/arch/arm/mach-omap2/board-zoom2.c
> > +++ b/arch/arm/mach-omap2/board-zoom2.c
> > @@ -24,10 +24,12 @@
> >  #include <mach/board-zoom.h>
> >
> >  #include "sdram-micron-mt46h32m32lf-6.h"
> > +#include "pm.h"
> >  #include "omap3-opp.h"
> >
> >  static void __init omap_zoom2_init_irq(void)
> >  {
> > +       omap3_pm_init_opp_table();
> >         omap2_init_common_hw(mt46h32m32lf6_sdrc_params,
> >                                  mt46h32m32lf6_sdrc_params,
> omap3_mpu_rate_table,
> >                                  omap3_dsp_rate_table,
> omap3_l3_rate_table);
> > diff --git a/arch/arm/mach-omap2/omap3-opp.h b/arch/arm/mach-
> omap2/omap3-opp.h
> > index 42557e1..994d8d4 100644
> > --- a/arch/arm/mach-omap2/omap3-opp.h
> > +++ b/arch/arm/mach-omap2/omap3-opp.h
> > @@ -3,60 +3,8 @@
> >
> >  #include <plat/omap-pm.h>
> >
> > -/* MPU speeds */
> > -#define S600M   600000000
> > -#define S550M   550000000
> > -#define S500M   500000000
> > -#define S250M   250000000
> > -#define S125M   125000000
> > -
> > -/* DSP speeds */
> > -#define S430M   430000000
> > -#define S400M   400000000
> > -#define S360M   360000000
> > -#define S180M   180000000
> > -#define S90M    90000000
> > -
> > -/* L3 speeds */
> > -#define S83M    83000000
> > -#define S166M   166000000
> > -
> > -static struct omap_opp omap3_mpu_rate_table[] = {
> > -       {0, 0, 0, 0},
> > -       /*OPP1*/
> > -       {true, S125M, VDD1_OPP1, 0x1E},
> > -       /*OPP2*/
> > -       {true, S250M, VDD1_OPP2, 0x26},
> > -       /*OPP3*/
> > -       {true, S500M, VDD1_OPP3, 0x30},
> > -       /*OPP4*/
> > -       {true, S550M, VDD1_OPP4, 0x36},
> > -       /*OPP5*/
> > -       {true, S600M, VDD1_OPP5, 0x3C},
> > -};
> > -
> > -static struct omap_opp omap3_l3_rate_table[] = {
> > -       {0, 0, 0, 0},
> > -       /*OPP1*/
> > -       {false, 0, VDD2_OPP1, 0x1E},
> > -       /*OPP2*/
> > -       {true, S83M, VDD2_OPP2, 0x24},
> > -       /*OPP3*/
> > -       {true, S166M, VDD2_OPP3, 0x2C},
> > -};
> > -
> > -static struct omap_opp omap3_dsp_rate_table[] = {
> > -       {0, 0, 0, 0},
> > -       /*OPP1*/
> > -       {true, S90M, VDD1_OPP1, 0x1E},
> > -       /*OPP2*/
> > -       {true, S180M, VDD1_OPP2, 0x26},
> > -       /*OPP3*/
> > -       {true, S360M, VDD1_OPP3, 0x30},
> > -       /*OPP4*/
> > -       {true, S400M, VDD1_OPP4, 0x36},
> > -       /*OPP5*/
> > -       {true, S430M, VDD1_OPP5, 0x3C},
> > -};
> > +extern struct omap_opp *omap3_mpu_rate_table;
> > +extern struct omap_opp *omap3_dsp_rate_table;
> > +extern struct omap_opp *omap3_l3_rate_table;
> >
> >  #endif
> > diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
> > index 7bc86b6..80a1c1d 100644
> > --- a/arch/arm/mach-omap2/pm.h
> > +++ b/arch/arm/mach-omap2/pm.h
> > @@ -58,6 +58,12 @@ static inline void omap3_pm_init_cpuidle(
> >  {
> >  }
> >  #endif
> > +/**
> > + * omap3_pm_init_opp_table - OMAP opp table lookup called after cpu is
> detected.
> > + * Initialize the basic opp table here, board files could choose to
> modify opp
> > + * table after the basic initialization
> > + */
> > +extern void omap3_pm_init_opp_table(void);
> >
> >  extern int resource_set_opp_level(int res, u32 target_level, int
> flags);
> >  extern int resource_access_opp_lock(int res, int delta);
> > diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> > index 627a509..e40a036 100644
> > --- a/arch/arm/mach-omap2/pm34xx.c
> > +++ b/arch/arm/mach-omap2/pm34xx.c
> > @@ -40,6 +40,7 @@
> >  #include <plat/dmtimer.h>
> >  #include <plat/usb.h>
> >
> > +#include <plat/opp.h>
> >  #include <plat/resource.h>
> >
> >  #include <asm/tlbflush.h>
> > @@ -52,6 +53,7 @@
> >  #include "prm.h"
> >  #include "pm.h"
> >  #include "sdrc.h"
> > +#include "omap3-opp.h"
> >
> >  static int regset_save_on_suspend;
> >
> > @@ -100,6 +102,49 @@ static struct prm_setup_vc prm_setup = {
> >         .vdd1_off = 0x00,       /* 0.6v */
> >  };
> >
> > +static struct omap_opp_def __initdata omap34xx_mpu_rate_table[] = {
> > +       /* OPP1 */
> > +       OMAP_OPP_DEF(true, 125000000, 975000),
> > +       /* OPP2 */
> > +       OMAP_OPP_DEF(true, 250000000, 1075000),
> > +       /* OPP3 */
> > +       OMAP_OPP_DEF(true, 500000000, 1200000),
> > +       /* OPP4 */
> > +       OMAP_OPP_DEF(true, 550000000, 1270000),
> > +       /* OPP5 */
> > +       OMAP_OPP_DEF(true, 600000000, 1350000),
> > +       OMAP_OPP_DEF_TERMINATOR
> > +};
> > +
> > +static struct omap_opp_def __initdata omap34xx_l3_rate_table[] = {
> > +       /* OPP1 */
> > +       OMAP_OPP_DEF(false, 0, 975000),
> > +       /* OPP2 */
> > +       OMAP_OPP_DEF(true, 83000000, 1050000),
> > +       /* OPP3 */
> > +       OMAP_OPP_DEF(true, 166000000, 1150000),
> > +       OMAP_OPP_DEF_TERMINATOR
> > +};
> > +
> > +static struct omap_opp_def __initdata omap34xx_dsp_rate_table[] = {
> > +       /* OPP1 */
> > +       OMAP_OPP_DEF(true, 90000000, 975000),
> > +       /* OPP2 */
> > +       OMAP_OPP_DEF(true, 180000000, 1075000),
> > +       /* OPP3 */
> > +       OMAP_OPP_DEF(true, 360000000, 1200000),
> > +       /* OPP4 */
> > +       OMAP_OPP_DEF(true, 400000000, 1270000),
> > +       /* OPP5 */
> > +       OMAP_OPP_DEF(true, 430000000, 1350000),
> > +       OMAP_OPP_DEF_TERMINATOR
> > +};
> > +
> > +/* OMAP3 Rate Table */
> > +struct omap_opp *omap3_mpu_rate_table;
> > +struct omap_opp *omap3_dsp_rate_table;
> > +struct omap_opp *omap3_l3_rate_table;
> > +
> >  static inline void omap3_per_save_context(void)
> >  {
> >         omap_gpio_save_context();
> > @@ -1248,6 +1293,26 @@ static void __init configure_vc(void)
> >         pm_dbg_regset_init(2);
> >  }
> >
> > +void __init omap3_pm_init_opp_table(void)
> > +{
> > +       int i;
> > +       struct omap_opp_def *omap34xx_opp_def_list[] = {
> > +               omap34xx_mpu_rate_table,
> > +               omap34xx_l3_rate_table,
> > +               omap34xx_dsp_rate_table
> > +       };
> > +       struct omap_opp **omap3_rate_tables[] = {
> > +               &omap3_mpu_rate_table,
> > +               &omap3_l3_rate_table,
> > +               &omap3_dsp_rate_table
> > +       };
> > +       for (i = 0; i < ARRAY_SIZE(omap3_rate_tables); i++) {
> > +               *omap3_rate_tables[i] =
> opp_init_list(omap34xx_opp_def_list[i]);
> > +               /* We dont want half configured system at the moment */
> > +               BUG_ON(IS_ERR(omap3_rate_tables[i]));
> > +       }
> > +}
> > +
> >  static int __init omap3_pm_early_init(void)
> >  {
> >         prm_clear_mod_reg_bits(OMAP3430_OFFMODE_POL, OMAP3430_GR_MOD,
> > diff --git a/arch/arm/plat-omap/include/plat/omap-pm.h b/arch/arm/plat-
> omap/include/plat/omap-pm.h
> > index 5dc2048..aa36339 100644
> > --- a/arch/arm/plat-omap/include/plat/omap-pm.h
> > +++ b/arch/arm/plat-omap/include/plat/omap-pm.h
> > @@ -18,22 +18,7 @@
> >  #include <linux/cpufreq.h>
> >
> >  #include "powerdomain.h"
> > -
> > -/**
> > - * struct omap_opp - clock frequency-to-OPP ID table for DSP, MPU
> > - * @enabled: enabled if true, disabled if false
> > - * @rate: target clock rate
> > - * @opp_id: OPP ID
> > - * @min_vdd: minimum VDD1 voltage (in millivolts) for this OPP
> > - *
> > - * Operating performance point data.  Can vary by OMAP chip and board.
> > - */
> > -struct omap_opp {
> > -       bool enabled;
> > -       unsigned long rate;
> > -       u8 opp_id;
> > -       u16 vsel;
> > -};
> > +#include <plat/opp.h>
> >
> >  extern struct omap_opp *mpu_opps;
> >  extern struct omap_opp *dsp_opps;
> > --
> > 1.6.3.3
>
> --
> Eduardo Valentin
Regards,
Nishanth Menon

RE: [PATCH 09/10 V4] omap3: pm: introduce 3630 opps

[Menon, Nishanth]

> From: Eduardo Valentin [mailto:eduardo.valentin@nokia.com]
> Sent: Friday, December 11, 2009 4:12 AM
> 
> Hello Nishanth,
> 
> On Wed, Dec 09, 2009 at 07:17:14AM +0100, ext Nishanth Menon wrote:
> > Introduce the OMAP3630 OPPs including the defined OPP tuples.
> >
> > Further information on OMAP3630 can be found here:
> >
> http://focus.ti.com/general/docs/wtbu/wtbuproductcontent.tsp?templateId=61
> 23&navigationId=12836&contentId=52606
> >
> > OMAP36xx family introduces:
> > VDD1 with 4 OPPs, of which OPP3 & 4 are available only on devices yet
> > to be introduced in 36xx family. Meanwhile, VDD2 has 2 opps.
> >
> > Device Support of OPPs->
> >            |<-3630-600->| (default)
> >            |<-      3630-800    ->| (device: TBD)
> >            |<-      3630-1000            ->| (device: TBD)
> > H/w OPP-> OPP50 OPP100       OPP-Turbo   OPP1G-SB
> > VDD1      OPP1  OPP2         OPP3        OPP4
> > VDD2      OPP1  OPP2         OPP2        OPP2
> >
> > Note:
> > a) TI h/w naming for OPPs are now standardized in terms of OPP50, 100,
> >    Turbo and SB. This maps as shown above to the opp IDs (s/w term).
> > b) For boards which need custom VDD1/2 OPPs, the opp table can be
> >    updated by the board file on a need basis after the
> >    omap3_pm_init_opp_table call. The OPPs introduced here are the
> >    official OPP table at this point in time.
> >
> 
> [ ... ]
> 
> >
> > +static struct omap_opp_def __initdata omap36xx_mpu_rate_table[] = {
> > +	/* OPP1 - OPP50 */
> > +	OMAP_OPP_DEF(true,  300000000, 930000),
> > +	/* OPP2 - OPP100 */
> > +	OMAP_OPP_DEF(true,  600000000, 1100000),
> > +	/* OPP3 - OPP-Turbo */
> > +	OMAP_OPP_DEF(false, 800000000, 1260000),
> > +	/* OPP4 - OPP-SB */
> > +	OMAP_OPP_DEF(false, 1000000000, 1310000),
> > +	OMAP_OPP_DEF_TERMINATOR
> 
> What else needs to be done in order to enable OPP-Turbo & OPP-SB ?

Sigh - long story ;) but in a short line: you just need to call opp_enable(freq) in your board file while we send a future patch to enable auto detection: DISCLAIMER: for 3630, there may not be a way to differentiate between devices other than some specific one - and board file may need to enable the opp that board has (shrug.. I know it is not a good idea - but there are some non-technical reasons involved  - still waiting for confirmation before being able to share additional info on list).

> 
> 
> > +};
> > +
> > +static struct omap_opp_def __initdata omap36xx_l3_rate_table[] = {
> > +	/* OPP1 - OPP50 */
> > +	OMAP_OPP_DEF(true, 100000000, 930000),
> > +	/* OPP2 - OPP100, OPP-Turbo, OPP-SB */
> > +	OMAP_OPP_DEF(true, 200000000, 1137500),
> > +	OMAP_OPP_DEF_TERMINATOR
> > +};
> > +
> > +static struct omap_opp_def __initdata omap36xx_dsp_rate_table[] = {
> > +	/* OPP1 - OPP50 */
> > +	OMAP_OPP_DEF(true,  260000000, 930000),
> > +	/* OPP2 - OPP100 */
> > +	OMAP_OPP_DEF(true,  520000000, 1100000),
> > +	/* OPP3 - OPP-Turbo */
> > +	OMAP_OPP_DEF(false, 660000000, 1260000),
> > +	/* OPP4 - OPP-SB */
> > +	OMAP_OPP_DEF(false, 875000000, 1310000),
> > +	OMAP_OPP_DEF_TERMINATOR
> > +};
> > +
> >  /* OMAP3 Rate Table */
> >  struct omap_opp *omap3_mpu_rate_table;
> >  struct omap_opp *omap3_dsp_rate_table;
> > @@ -1296,18 +1328,27 @@ static void __init configure_vc(void)
> >  void __init omap3_pm_init_opp_table(void)
> >  {
> >  	int i;
> > +	struct omap_opp_def **omap3_opp_def_list;
> >  	struct omap_opp_def *omap34xx_opp_def_list[] = {
> >  		omap34xx_mpu_rate_table,
> >  		omap34xx_l3_rate_table,
> >  		omap34xx_dsp_rate_table
> >  	};
> > +	struct omap_opp_def *omap36xx_opp_def_list[] = {
> > +		omap36xx_mpu_rate_table,
> > +		omap36xx_l3_rate_table,
> > +		omap36xx_dsp_rate_table
> > +	};
> >  	struct omap_opp **omap3_rate_tables[] = {
> >  		&omap3_mpu_rate_table,
> >  		&omap3_l3_rate_table,
> >  		&omap3_dsp_rate_table
> >  	};
> > +
> > +	omap3_opp_def_list = cpu_is_omap3630() ? omap36xx_opp_def_list :
> > +				omap34xx_opp_def_list;
> >  	for (i = 0; i < ARRAY_SIZE(omap3_rate_tables); i++) {
> > -		*omap3_rate_tables[i] =
> opp_init_list(omap34xx_opp_def_list[i]);
> > +		*omap3_rate_tables[i] = opp_init_list(omap3_opp_def_list[i]);
> >  		/* We dont want half configured system at the moment */
> >  		BUG_ON(IS_ERR(omap3_rate_tables[i]));
> >  	}
> > --
> > 1.6.3.3
> 
> --
> Eduardo Valentin

RE: [PATCH 02/10 V4] omap3: pm: introduce opp accessor functions

[Menon, Nishanth]

Eduardo,
> From: Eduardo Valentin [mailto:eduardo.valentin@nokia.com]
> Sent: Friday, December 11, 2009 3:19 AM
> 
> On Fri, Dec 11, 2009 at 01:41:46AM +0100, ext Nishanth Menon wrote:
> > Kevin Hilman had written, on 12/10/2009 05:25 PM, the following:
> > Thanks for the acks..
> >
> > > Nishanth Menon <nm@ti.com> writes:
> > [...]
> >
> > >> diff --git a/arch/arm/plat-omap/include/plat/opp.h b/arch/arm/plat-
> omap/include/plat/opp.h
> > >> new file mode 100644
> > >> index 0000000..341c02b
> >
> > [...]
> >
> > >> +/**
> > >> + * struct omap_opp - OMAP OPP description structure
> > >> + * @enabled: true/false - marking this OPP as enabled/disabled
> > >> + * @rate:    Frequency in hertz
> > >> + * @opp_id:  (DEPRECATED) opp identifier
> > >> + * @vsel:    Voltage in volt processor level(this usage is
> > >> + *           DEPRECATED to use Voltage in microvolts in future)
> > >> + *           uV = ((vsel * 12.5) + 600) * 1000
> > >> + *
> > >> + * This structure stores the OPP information for a given domain.
> > >> + * Due to legacy reasons, this structure is currently exposed and
> > >> + * will soon be removed elsewhere and will only be used as a handle
> > >> + * from the OPP internal referencing mechanism
> > >> + */
> > >> +struct omap_opp {
> > >> +     bool enabled;
> > >> +     unsigned long rate;
> > >> +     u8 opp_id __deprecated;
> > >> +     u16 vsel;
> > >
> > > How about we add 'u32 voltage' here and mark vsel as __deprecated.
> Then
> > > we no longer need both an 'struct omap_opp' and a 'struct
> omap_opp_def'.
> > >
> > > Or even better, with the uv <--> vsel conversion macros you added,
> > > couldn't we alrady define the OPPs in terms of voltage, and drop the
> > > vsel already?
> >
> > we should do that once we fix SR + resource34xx (underworks) - they
> > directly use them and I kept my "status quo" rule switch on ;). Once it
> > is done, vsel becomes voltage and an unsigned long. and we can manage it
> > inside opp.c anyway we choose. For this starting set, I dont think we
> > should do this.
> 
> I'm OK if you have the plan to do it in steps. But might be useful to have
> some
> REVISIT / TODO comment on top of things you know you are going to change
> afterwards.
> 
> It is not mandatory, but it helps to keep track of what is in your plans.

Is the following not good enough?

> >> + * @vsel:    Voltage in volt processor level(this usage is
> >> + *           DEPRECATED to use Voltage in microvolts in future)
                   ^^^^^^^^^^^
> >> + *           uV = ((vsel * 12.5) + 600) * 1000
> >> + *
> >> + * This structure stores the OPP information for a given domain.
> >> + * Due to legacy reasons, this structure is currently exposed and
> >> + * will soon be removed elsewhere and will only be used as a handle
> >> + * from the OPP internal referencing mechanism

Yes, I believe I have been liberal in sprinkling TODOs and __deprecated in 
my patches.. but let me know if I missed any specifics..

> 
> >
> > [...]
> > >
> > >> +/**
> > >> + * opp_find_freq_exact() - search for an exact frequency
> > >> + * @oppl:    OPP list
> > >> + * @freq:    frequency to search for
> > >> + * @enabled: enabled/disabled OPP to search for
> > >> + *
> > >> + * searches for the match in the opp list and returns handle to the
> matching
> > >> + * opp if found, else returns ERR_PTR in case of error and should be
> handled
> > >> + * using IS_ERR.
> > >> + *
> > >> + * Note enabled is a modifier for the search. if enabled=true, then
> the match is
> > >> + * for exact matching frequency and is enabled. if true, the match
> is for exact
> > >> + * frequency which is disabled.
> > >> + */
> > >> +struct omap_opp *opp_find_freq_exact(struct omap_opp *oppl,
> > >> +                                  unsigned long freq, bool enabled);
> > >
> > > ack
> > >
> > > I think we could drop the _exact, and just call it opp_find_freq(),
> but I'm
> > > ok either way.
> > shrug.. kinda matches with _approx .. it improves readability esp when
> > people look at a usage code 6 months from now and question what
> > find_freq is doing and get confused about freq_approx
> >
> > [...]
> >
> > >> +/**
> > >> + * struct omap_opp_def - OMAP OPP Definition
> > >> + * @enabled: True/false - is this OPP enabled/disabled by default
> > >> + * @freq:    Frequency in hertz corresponding to this OPP
> > >> + * @u_volt:  Nominal voltage in microvolts corresponding to this OPP
> > >> + *
> > >> + * OMAP SOCs have a standard set of tuples consisting of frequency
> and voltage
> > >> + * pairs that the device will support per voltage domain. This is
> called
> > >> + * Operating Points or OPP. The actual definitions of OMAP Operating
> Points
> > >> + * varies over silicon within the same family of devices. For a
> specific
> > >> + * domain, you can have a set of {frequency, voltage} pairs and this
> is denoted
> > >> + * by an array of omap_opp_def. As the kernel boots and more
> information is
> > >> + * available, a set of these are activated based on the precise
> nature of
> > >> + * device the kernel boots up on. It is interesting to remember that
> each IP
> > >> + * which belongs to a voltage domain may define their own set of
> OPPs on top
> > >> + * of this - but this is handled by the appropriate driver.
> > >> + */
> > >> +struct omap_opp_def {
> > >> +     bool enabled;
> > >> +     unsigned long freq;
> > >> +     u32 u_volt;
> > Comment to self: I should really make the u32 as unsigned long to be in
> > sync with what is used elsewhere..(get_voltage)
> >
> > >> +};
> > >
> > > See above comment on 'struct omap_opp'.  I think these two should be
> > > combined.
> > >
> > > I think the initial intent of having them separated so that the
> > > internal struct of 'struct omap_opp' could eventually move to the C
> > > file was the original intent, but I think it aids readability to just
> > > have a single OPP struct.
> >
> > In a few weeks we wont have the struct omap_opp exposed out(once all the
> > cleanups are done).. at that point, how would one define an OPP and
> > expect to get an handle which they cannot manipulate?
> >
> > >
> > >> +/* Initialization wrapper */
> > >> +#define OMAP_OPP_DEF(_enabled, _freq, _uv)   \
> > >> +{                                            \
> > >> +     .enabled        = _enabled,             \
> > >> +     .freq           = _freq,                \
> > >> +     .u_volt         = _uv,                  \
> > >> +}
> > >
> > > nice
> > >
> > >> +/* Terminator for the initialization list */
> > >> +#define OMAP_OPP_DEF_TERMINATOR OMAP_OPP_DEF(0, 0, 0)
> > >
> > > I'd just drop this and use OMAP_OPP_DEF(0, 0, 0) directly in
> > > the table.
> >
> > Am ok with either (I dont like additional #defs). but terminator helps
> > redability a bit though (debatable).. any reasons why u'd like it 0,0,0?
> > >
> > >> +/**
> > >> + * opp_init_list() - Initialize an opp list from the opp definitions
> > >> + * @opp_defs:        Initial opp definitions to create the list.
> > >> + *
> > >> + * This function creates a list of opp definitions and returns a
> handle.
> > >> + * This list can be used to further validation/search/modifications.
> New
> > >> + * opp entries can be added to this list by using opp_add().
> > >> + *
> > >> + * In the case of error, ERR_PTR is returned to the caller and
> should be
> > >> + * appropriately handled with IS_ERR.
> > >> + */
> > >> +struct omap_opp __init *opp_init_list(const struct omap_opp_def
> *opp_defs);
> > >
> > > My original suggestion was that opp_init_list() simply creates a new
> > > but empty list.  Adding OPPs should be done using opp_add().
> > >
> > > I guess I'm OK with having the 'bulk add' feature of init_list() but
> > > would rather see a single way to add OPPs.
> > Reasons why to have a buld add feature in init:
> > a) There is opp_add below which allows u to add single opp
> > b) In terms of walk thru code duplication (once this gets used accross
> > OMAPs) it is essentially the same thing we do (add each OPP definition
> > for a domain)..
> > c) you dont incur function call latencies. (ok not a big deal.. but
> still).
> >
> > >
> > >> +/**
> > >> + * opp_add()  - Add an OPP table from a table definitions
> > >> + * @oppl:    List to add the OPP to
> > >> + * @opp_def: omap_opp_def to describe the OPP which we want to add
> to list.
> > >> + *
> > >> + * This function adds an opp definition to the opp list and returns
> > >> + * a handle representing the new OPP list. This handle is then used
> for further
> > >> + * validation, search, modification operations on the OPP list.
> > >> + *
> > >> + * This function returns the pointer to the allocated list through
> oppl if
> > >> + * success, else corresponding ERR_PTR value. Caller should NOT free
> the oppl.
> > >> + * opps_defs can be freed after use.
> > >> + *
> > >> + * NOTE: caller should assume that on success, oppl is probably
> populated with
> > >> + * a new handle and the new handle should be used for further
> referencing
> > >> + */
> > >> +struct omap_opp *opp_add(struct omap_opp *oppl,
> > >> +                      const struct omap_opp_def *opp_def);
> > >
> > > c.f. proposal to drop omap_opp_def.
> > explained above.
> >
> > >
> > > otherwise, ack.
> > >
> >
> > --
> > Regards,
> > Nishanth Menon
> 
> --
> Eduardo Valentin

Regards,
Nishanth Menon

Re: [PATCH 02/10 V4] omap3: pm: introduce opp accessor functions

[Kevin Hilman]

Nishanth Menon <nm@ti.com> writes:

> Kevin Hilman had written, on 12/10/2009 05:25 PM, the following:
> Thanks for the acks..
>
>> Nishanth Menon <nm@ti.com> writes:
> [...]
>
>>> diff --git a/arch/arm/plat-omap/include/plat/opp.h b/arch/arm/plat-omap/include/plat/opp.h
>>> new file mode 100644
>>> index 0000000..341c02b
>
> [...]
>
>>> +/**
>>> + * struct omap_opp - OMAP OPP description structure
>>> + * @enabled: true/false - marking this OPP as enabled/disabled
>>> + * @rate:    Frequency in hertz
>>> + * @opp_id:  (DEPRECATED) opp identifier
>>> + * @vsel:    Voltage in volt processor level(this usage is
>>> + *           DEPRECATED to use Voltage in microvolts in future)
>>> + *           uV = ((vsel * 12.5) + 600) * 1000
>>> + *
>>> + * This structure stores the OPP information for a given domain.
>>> + * Due to legacy reasons, this structure is currently exposed and
>>> + * will soon be removed elsewhere and will only be used as a handle
>>> + * from the OPP internal referencing mechanism
>>> + */
>>> +struct omap_opp {
>>> +     bool enabled;
>>> +     unsigned long rate;
>>> +     u8 opp_id __deprecated;
>>> +     u16 vsel;
>>
>> How about we add 'u32 voltage' here and mark vsel as __deprecated.  Then
>> we no longer need both an 'struct omap_opp' and a 'struct omap_opp_def'.
>>
>> Or even better, with the uv <--> vsel conversion macros you added,
>> couldn't we alrady define the OPPs in terms of voltage, and drop the
>> vsel already?
>
> we should do that once we fix SR + resource34xx (underworks) - they
> directly use them and I kept my "status quo" rule switch on ;). Once
> it is done, vsel becomes voltage and an unsigned long. 

ok, should still flag vsel with __deprecated though.

> and we can
> manage it inside opp.c anyway we choose. For this starting set, I dont
> think we should do this.
>
> [...]
>>
>>> +/**
>>> + * opp_find_freq_exact() - search for an exact frequency
>>> + * @oppl:    OPP list
>>> + * @freq:    frequency to search for
>>> + * @enabled: enabled/disabled OPP to search for
>>> + *
>>> + * searches for the match in the opp list and returns handle to the matching
>>> + * opp if found, else returns ERR_PTR in case of error and should be handled
>>> + * using IS_ERR.
>>> + *
>>> + * Note enabled is a modifier for the search. if enabled=true, then the match is
>>> + * for exact matching frequency and is enabled. if true, the match is for exact
>>> + * frequency which is disabled.
>>> + */
>>> +struct omap_opp *opp_find_freq_exact(struct omap_opp *oppl,
>>> +                                  unsigned long freq, bool enabled);
>>
>> ack
>>
>> I think we could drop the _exact, and just call it opp_find_freq(), but I'm
>> ok either way.
> shrug.. kinda matches with _approx .. it improves readability esp when
> people look at a usage code 6 months from now and question what
> find_freq is doing and get confused about freq_approx

ok

> [...]
>
>>> +/**
>>> + * struct omap_opp_def - OMAP OPP Definition
>>> + * @enabled: True/false - is this OPP enabled/disabled by default
>>> + * @freq:    Frequency in hertz corresponding to this OPP
>>> + * @u_volt:  Nominal voltage in microvolts corresponding to this OPP
>>> + *
>>> + * OMAP SOCs have a standard set of tuples consisting of frequency and voltage
>>> + * pairs that the device will support per voltage domain. This is called
>>> + * Operating Points or OPP. The actual definitions of OMAP Operating Points
>>> + * varies over silicon within the same family of devices. For a specific
>>> + * domain, you can have a set of {frequency, voltage} pairs and this is denoted
>>> + * by an array of omap_opp_def. As the kernel boots and more information is
>>> + * available, a set of these are activated based on the precise nature of
>>> + * device the kernel boots up on. It is interesting to remember that each IP
>>> + * which belongs to a voltage domain may define their own set of OPPs on top
>>> + * of this - but this is handled by the appropriate driver.
>>> + */
>>> +struct omap_opp_def {
>>> +     bool enabled;
>>> +     unsigned long freq;
>>> +     u32 u_volt;
> Comment to self: I should really make the u32 as unsigned long to be
> in sync with what is used elsewhere..(get_voltage)
>
>>> +};
>>
>> See above comment on 'struct omap_opp'.  I think these two should be
>> combined.
>>
>> I think the initial intent of having them separated so that the
>> internal struct of 'struct omap_opp' could eventually move to the C
>> file was the original intent, but I think it aids readability to just
>> have a single OPP struct.
>
> In a few weeks we wont have the struct omap_opp exposed out(once all
> the cleanups are done).. at that point, how would one define an OPP
> and expect to get an handle which they cannot manipulate?

Understood.  But, when we get to that point, we'll have a 'struct
omap_opp' and a 'struct omap_opp_def' which are identical.
Personally, I'd rather just keep a single struct defined in the
header.  

Since this is core OMAP code, I'm not too concerned (anymore) about
direct manipulation of OPP structs since I will NAK any such changes
anyways.

Primarily, I see having two different structs for essentially the same
thing being a readability issue.

>>
>>> +/* Initialization wrapper */
>>> +#define OMAP_OPP_DEF(_enabled, _freq, _uv)   \
>>> +{                                            \
>>> +     .enabled        = _enabled,             \
>>> +     .freq           = _freq,                \
>>> +     .u_volt         = _uv,                  \
>>> +}
>>
>> nice
>>
>>> +/* Terminator for the initialization list */
>>> +#define OMAP_OPP_DEF_TERMINATOR OMAP_OPP_DEF(0, 0, 0)
>>
>> I'd just drop this and use OMAP_OPP_DEF(0, 0, 0) directly in
>> the table.
>
> Am ok with either (I dont like additional #defs). but terminator helps
> redability a bit though (debatable).. any reasons why u'd like it
> 0,0,0?

Personally I think having NULL or zero terminators is common enough that when
someone sees zeros they assume it's a terminator.  Having another #define means
you have to go and look what the terminator is.

Anyways, this is a minor point and just my opinion.  You can make the
final decision.

>>
>>> +/**
>>> + * opp_init_list() - Initialize an opp list from the opp definitions
>>> + * @opp_defs:        Initial opp definitions to create the list.
>>> + *
>>> + * This function creates a list of opp definitions and returns a handle.
>>> + * This list can be used to further validation/search/modifications. New
>>> + * opp entries can be added to this list by using opp_add().
>>> + *
>>> + * In the case of error, ERR_PTR is returned to the caller and should be
>>> + * appropriately handled with IS_ERR.
>>> + */
>>> +struct omap_opp __init *opp_init_list(const struct omap_opp_def *opp_defs);
>>
>> My original suggestion was that opp_init_list() simply creates a new
>> but empty list.  Adding OPPs should be done using opp_add().
>>
>> I guess I'm OK with having the 'bulk add' feature of init_list() but
>> would rather see a single way to add OPPs.
>
> Reasons why to have a buld add feature in init:
> a) There is opp_add below which allows u to add single opp
> b) In terms of walk thru code duplication (once this gets used accross
> OMAPs) it is essentially the same thing we do (add each OPP definition
> for a domain)..
> c) you dont incur function call latencies. (ok not a big deal.. but still).

Understood, but I still prefer without the bulk add, but again it's a
very minor issue to me and I'll leave it to you to make the final call.

>>
>>> +/**
>>> + * opp_add()  - Add an OPP table from a table definitions
>>> + * @oppl:    List to add the OPP to
>>> + * @opp_def: omap_opp_def to describe the OPP which we want to add to list.
>>> + *
>>> + * This function adds an opp definition to the opp list and returns
>>> + * a handle representing the new OPP list. This handle is then used for further
>>> + * validation, search, modification operations on the OPP list.
>>> + *
>>> + * This function returns the pointer to the allocated list through oppl if
>>> + * success, else corresponding ERR_PTR value. Caller should NOT free the oppl.
>>> + * opps_defs can be freed after use.
>>> + *
>>> + * NOTE: caller should assume that on success, oppl is probably populated with
>>> + * a new handle and the new handle should be used for further referencing
>>> + */
>>> +struct omap_opp *opp_add(struct omap_opp *oppl,
>>> +                      const struct omap_opp_def *opp_def);
>>
>> c.f. proposal to drop omap_opp_def.
> explained above.
>
>>
>> otherwise, ack.
>>
>
> -- 
> Regards,
> Nishanth Menon

Kevin

RE: [PATCH 02/10 V4] omap3: pm: introduce opp accessor functions

[Menon, Nishanth]

> From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
> Sent: Friday, December 11, 2009 9:48 AM
Thanks for your comments. My views follow


[...]
> >>> + * This structure stores the OPP information for a given domain.
> >>> + * Due to legacy reasons, this structure is currently exposed and
> >>> + * will soon be removed elsewhere and will only be used as a handle
> >>> + * from the OPP internal referencing mechanism
> >>> + */
> >>> +struct omap_opp {
> >>> +     bool enabled;
> >>> +     unsigned long rate;
> >>> +     u8 opp_id __deprecated;
> >>> +     u16 vsel;
> >>
> >> How about we add 'u32 voltage' here and mark vsel as __deprecated.
> Then
> >> we no longer need both an 'struct omap_opp' and a 'struct omap_opp_def'.
> >>
> >> Or even better, with the uv <--> vsel conversion macros you added,
> >> couldn't we alrady define the OPPs in terms of voltage, and drop the
> >> vsel already?
> >
> > we should do that once we fix SR + resource34xx (underworks) - they
> > directly use them and I kept my "status quo" rule switch on ;). Once
> > it is done, vsel becomes voltage and an unsigned long.
> 
> ok, should still flag vsel with __deprecated though.
ACK.
[...]
> 
> > [...]
> >
> >>> +/**
> >>> + * struct omap_opp_def - OMAP OPP Definition
> >>> + * @enabled: True/false - is this OPP enabled/disabled by default
> >>> + * @freq:    Frequency in hertz corresponding to this OPP
> >>> + * @u_volt:  Nominal voltage in microvolts corresponding to this OPP
> >>> + *
> >>> + * OMAP SOCs have a standard set of tuples consisting of frequency
> and voltage
> >>> + * pairs that the device will support per voltage domain. This is
> called
> >>> + * Operating Points or OPP. The actual definitions of OMAP Operating
> Points
> >>> + * varies over silicon within the same family of devices. For a
> specific
> >>> + * domain, you can have a set of {frequency, voltage} pairs and this
> is denoted
> >>> + * by an array of omap_opp_def. As the kernel boots and more
> information is
> >>> + * available, a set of these are activated based on the precise
> nature of
> >>> + * device the kernel boots up on. It is interesting to remember that
> each IP
> >>> + * which belongs to a voltage domain may define their own set of OPPs
> on top
> >>> + * of this - but this is handled by the appropriate driver.
> >>> + */
> >>> +struct omap_opp_def {
> >>> +     bool enabled;
> >>> +     unsigned long freq;
> >>> +     u32 u_volt;
> > Comment to self: I should really make the u32 as unsigned long to be
> > in sync with what is used elsewhere..(get_voltage)
> >
> >>> +};
> >>
> >> See above comment on 'struct omap_opp'.  I think these two should be
> >> combined.
> >>
> >> I think the initial intent of having them separated so that the
> >> internal struct of 'struct omap_opp' could eventually move to the C
> >> file was the original intent, but I think it aids readability to just
> >> have a single OPP struct.
> >
> > In a few weeks we wont have the struct omap_opp exposed out(once all
> > the cleanups are done).. at that point, how would one define an OPP
> > and expect to get an handle which they cannot manipulate?
> 
> Understood.  But, when we get to that point, we'll have a 'struct
> omap_opp' and a 'struct omap_opp_def' which are identical.
Is'nt this a temporary status? Once we get there, here is how it might look like:

Omap_opp as a list:
Voltage
frequency
pointer to next
OR:
Omap_opp might be a flexible array

OR
Omap_opp might be something altogether different like a hash table or something.

Omap_def wont change.

> Personally, I'd rather just keep a single struct defined in the
> header.
I think that is an invitation for something slipping through.. esp in private trees.. and end up with variant code bases.

> 
> Since this is core OMAP code, I'm not too concerned (anymore) about
> direct manipulation of OPP structs since I will NAK any such changes
> anyways.

Trouble I will face is in private trees and incapability of those patches
Being send back upstream if we allow direct accesses (I know this will not
Prevent people from exposing omap_opp and then doing what they please in
Private trees, but why make it easy?).
 
> 
> Primarily, I see having two different structs for essentially the same
> thing being a readability issue.
It is not. It is meant for two different things:
a) Def: register the OPPs that the configuration has. 
b) omap_opp: opp query/operation -> it can be whatever we choose it to be.

These two can independently be modified without constraints to either.

> 
> >>
> >>> +/* Initialization wrapper */
> >>> +#define OMAP_OPP_DEF(_enabled, _freq, _uv)   \
> >>> +{                                            \
> >>> +     .enabled        = _enabled,             \
> >>> +     .freq           = _freq,                \
> >>> +     .u_volt         = _uv,                  \
> >>> +}
> >>
> >> nice
> >>
> >>> +/* Terminator for the initialization list */
> >>> +#define OMAP_OPP_DEF_TERMINATOR OMAP_OPP_DEF(0, 0, 0)
> >>
> >> I'd just drop this and use OMAP_OPP_DEF(0, 0, 0) directly in
> >> the table.
> >
> > Am ok with either (I dont like additional #defs). but terminator helps
> > redability a bit though (debatable).. any reasons why u'd like it
> > 0,0,0?
> 
> Personally I think having NULL or zero terminators is common enough that
> when
> someone sees zeros they assume it's a terminator.  Having another #define
> means
> you have to go and look what the terminator is.
> 
> Anyways, this is a minor point and just my opinion.  You can make the
> final decision.
Valid point.. I am for 0s. yeah, I prefer not to have #defs if I can avoid it, it ends up having the coder to look twice to figure out what is happening..

> 
> >>
> >>> +/**
> >>> + * opp_init_list() - Initialize an opp list from the opp definitions
> >>> + * @opp_defs:        Initial opp definitions to create the list.
> >>> + *
> >>> + * This function creates a list of opp definitions and returns a
> handle.
> >>> + * This list can be used to further validation/search/modifications.
> New
> >>> + * opp entries can be added to this list by using opp_add().
> >>> + *
> >>> + * In the case of error, ERR_PTR is returned to the caller and should
> be
> >>> + * appropriately handled with IS_ERR.
> >>> + */
> >>> +struct omap_opp __init *opp_init_list(const struct omap_opp_def
> *opp_defs);
> >>
> >> My original suggestion was that opp_init_list() simply creates a new
> >> but empty list.  Adding OPPs should be done using opp_add().
> >>
> >> I guess I'm OK with having the 'bulk add' feature of init_list() but
> >> would rather see a single way to add OPPs.
> >
> > Reasons why to have a buld add feature in init:
> > a) There is opp_add below which allows u to add single opp
> > b) In terms of walk thru code duplication (once this gets used accross
> > OMAPs) it is essentially the same thing we do (add each OPP definition
> > for a domain)..
> > c) you dont incur function call latencies. (ok not a big deal.. but
> still).
> 
> Understood, but I still prefer without the bulk add, but again it's a
> very minor issue to me and I'll leave it to you to make the final call.

Going with bulk add if there are no further disapprovals from others.

Regards,
Nishanth Menon

Re: [PATCH 02/10 V4] omap3: pm: introduce opp accessor functions

[Kevin Hilman]

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

[...]

>> >>> +/**
>> >>> + * struct omap_opp_def - OMAP OPP Definition
>> >>> + * @enabled: True/false - is this OPP enabled/disabled by default
>> >>> + * @freq:    Frequency in hertz corresponding to this OPP
>> >>> + * @u_volt:  Nominal voltage in microvolts corresponding to this OPP
>> >>> + *
>> >>> + * OMAP SOCs have a standard set of tuples consisting of frequency
>> and voltage
>> >>> + * pairs that the device will support per voltage domain. This is
>> called
>> >>> + * Operating Points or OPP. The actual definitions of OMAP Operating
>> Points
>> >>> + * varies over silicon within the same family of devices. For a
>> specific
>> >>> + * domain, you can have a set of {frequency, voltage} pairs and this
>> is denoted
>> >>> + * by an array of omap_opp_def. As the kernel boots and more
>> information is
>> >>> + * available, a set of these are activated based on the precise
>> nature of
>> >>> + * device the kernel boots up on. It is interesting to remember that
>> each IP
>> >>> + * which belongs to a voltage domain may define their own set of OPPs
>> on top
>> >>> + * of this - but this is handled by the appropriate driver.
>> >>> + */
>> >>> +struct omap_opp_def {
>> >>> +     bool enabled;
>> >>> +     unsigned long freq;
>> >>> +     u32 u_volt;
>> > Comment to self: I should really make the u32 as unsigned long to be
>> > in sync with what is used elsewhere..(get_voltage)
>> >
>> >>> +};
>> >>
>> >> See above comment on 'struct omap_opp'.  I think these two should be
>> >> combined.
>> >>
>> >> I think the initial intent of having them separated so that the
>> >> internal struct of 'struct omap_opp' could eventually move to the C
>> >> file was the original intent, but I think it aids readability to just
>> >> have a single OPP struct.
>> >
>> > In a few weeks we wont have the struct omap_opp exposed out(once all
>> > the cleanups are done).. at that point, how would one define an OPP
>> > and expect to get an handle which they cannot manipulate?
>> 
>> Understood.  But, when we get to that point, we'll have a 'struct
>> omap_opp' and a 'struct omap_opp_def' which are identical.
> Is'nt this a temporary status? Once we get there, here is how it might look like:
>
> Omap_opp as a list:
> Voltage
> frequency
> pointer to next
> OR:
> Omap_opp might be a flexible array
>
> OR
> Omap_opp might be something altogether different like a hash table or something.
>
> Omap_def wont change.

Good point.

>> Personally, I'd rather just keep a single struct defined in the
>> header.
> I think that is an invitation for something slipping through.. esp in private trees.. and end up with variant code bases.
>
>> 
>> Since this is core OMAP code, I'm not too concerned (anymore) about
>> direct manipulation of OPP structs since I will NAK any such changes
>> anyways.
>
> Trouble I will face is in private trees and incapability of those patches
> Being send back upstream if we allow direct accesses (I know this will not
> Prevent people from exposing omap_opp and then doing what they please in
> Private trees, but why make it easy?).

Agreed.

>> 
>> Primarily, I see having two different structs for essentially the same
>> thing being a readability issue.
> It is not. It is meant for two different things:
> a) Def: register the OPPs that the configuration has. 
> b) omap_opp: opp query/operation -> it can be whatever we choose it to be.
>
> These two can independently be modified without constraints to either.

OK, I'm convinced.

No further objections your honor.  ;)

[...]

>> 
>> Understood, but I still prefer without the bulk add, but again it's a
>> very minor issue to me and I'll leave it to you to make the final call.
>
> Going with bulk add if there are no further disapprovals from others.
>

ok

Kevin

RE: [PATCH 02/10 V4] omap3: pm: introduce opp accessor functions

[Paul Walmsley]

Hello Nishanth,

Following are some comments on your OPP code (revision five).

While preparing them, I put together some experimental patches to
verify that the comments resulted in cleaner code.  They will be sent
as separate messages.  You might find them useful.  If so, you're
welcome to integrate them to your code.


- Paul


General comments:

- The OPP code has hardcoded dependencies on the TWL/TPS
  Triton2/Gaia/Reno series of chips.  Please move the uv_to_vsel() and 
  vsel_to_uv() functions off to a
  separate file, and rename them to mark them as being
  TWL/TPS-specific, for example, "twl_uv_to_vsel()" etc.  These
  functions will probably not be deprecatable because they will be
  needed by the TWL/TPS code to program the PMIC correctly.  So it
  also makes sense to remove the 'deprecated' markings.

- Please remove the vsel data out of the OPP layer.  The OPP code
  should be independent of the PMIC.

- Let's avoid the "initial terminators" hack and convert all of the
  code that tries to access the *_opps[] arrays directly to use
  accessor functions.  Judging by a grep through the codebase, it
  seems that SRF and SmartReflex are the only offenders here.

- Your patches don't build with !CONFIG_PM.  Please fix.

- The vsel rounding currently handled in omap_opp_populate() should be
  handled in uv_to_vsel(), lest other code that uses uv_to_vsel()
  get different answers.

- In pm34xx.c:omap34xx_l3_rate_table(), why is OPP1 defined at 0 Hz -
  shouldn't this be 41.5MHz?

- Since the clock framework no longer has anything to do with
  generating the cpufreq_frequency_table for OMAP3, please move
  clock34xx.c:omap2_clk_init_cpufreq_table() to a more logical place
  such as opp.c, and rename the function appropriately -- perhaps
  something like opp_init_cpufreq_table() ?  Probably
  plat-omap/cpu-omap.c:omap_cpu_init() will need a little extra "if
  (cpu_is_*)" code in the short term until the OMAP2 OPPs are
  converted to use the OPP layer also.

- Rather than using opp_find_freq_approx() in
  omap2_clk_init_cpufreq_table(), it seems better to just create a
  function that returns an array of data structures appropriately
  constructed for cpu-omap.c.  This can be rolled into
  opp_init_cpufreq_table(), since CPUFreq will be the only user.


Simplicity/readability comments:

- In opp.c:opp_add(), constants like '3' or '2' in lines like these:

	oppr = kmalloc(sizeof(struct omap_opp) * (n + 3), GFP_KERNEL);

  need to be documented.  In the above case, I assume this is the
  "initial terminator" plus the new entry, plus the ending terminator.
  This should be documented, at least with a comment, and ideally in
  one or more macros with comments.

- Please split opp_find_freq_approx() into two functions.  This seems
  simpler and more readable.  Consider using terms like "floor" and
  "ceil" in the function names, since they'll be instantly
  recognizable to most programmers with POSIX experience (libm).


A few minor comments (but important nonetheless):

- In opp.c, you have two different kinds of "invalid parameter" error
  messages:

	pr_err("%s: Invalid parameters being passed\n", __func__);

  and

	pr_err("%s: Invalid params being passed\n", __func__);

  This consumes memory unnecessarily, since both strings will have to
  be stored - this can be avoided if you use the same string text.
  Also, please use WARN(), rather than pr_err(), in these cases.
  WARN() will provide a stack backtrace which will help users
  determine which function caused the error.

- There are lots of uses of unlikely() in non-fast path code.  These seem
  unnecessary.  

- Good kerneldoc - this really helps.  Please move the kerneldoc
  comments from the plat/opp.h file to the plat-omap/opp.c file.  The
  usual rationale for putting function comments in the .h file is if
  the .h file represents an interface that can be implemented by a
  variety of possible .c files.  That isn't the case here - there
  should be only one OPP code layer for OMAP.

- Please consider avoiding words like "coz" in the patch descriptions.
  The full expansion is not too hard to type and it will make the
  commit messages easier to understand for people who are less
  familiar with English.

- You've added several BUG_ON()s that don't appear to be necessary.
  The usual kernel practice is to only use BUG_ON()s for critical
  errors.  For example, BUG_ON()s in these functions should be
  converted to either return an error code or to WARN():
  opp_to_freq(); freq_to_opp().  Others should probably be removed
  also, but given the current state of the SRF, I'm not sure I'd
  recommend removing them.

- Consider simplifying the name of opp_get_opp_count() to just
  opp_count_opps().

- While you're touching the SRF code, could you also rename the SRF
  freq_to_opp() and opp_to_freq() to "freq_to_opp_id()" and
  "opp_id_to_freq()", to help clarify what these functions do?

- Consider the use of kzalloc() rather than kmalloc() in the
  opp.c code.  The advantage of kzalloc() is that, if the structure is
  are changed, whoever changes them does not have to worry about
  initializing the structure fields to zero, which can save someone
  else some debugging time.

- I'll just note here that we'll need to extend this code further in a
  few respects that we've discussed before: OMAP1/2 support and
  VDD1<->VDD2 OPP dependencies are ones that come to mind.  Not that we
  have to grapple with them now, but as you revise your code, perhaps you
  can keep these in mind.


- Paul

RE: [PATCH 02/10 V4] omap3: pm: introduce opp accessor functions

[Paul Walmsley]

On Thu, 17 Dec 2009, Paul Walmsley wrote:

> Following are some comments on your OPP code (revision five).
> 
> While preparing them, I put together some experimental patches to
> verify that the comments resulted in cleaner code.  They will be sent
> as separate messages.  You might find them useful.  If so, you're
> welcome to integrate them to your code.

Just wanted to clarify that the patches only implement a subset of the 
comments from this mail - mostly just the items that I wasn't sure how 
they would turn out.


- Paul