[PATCH 00/13] OMAP: Basic DVFS framework

[PATCH 00/13] OMAP: Basic DVFS framework

[Thara Gopinath]

From: thara gopinath <thara@ti.com>

This patch series introduces support  for support of Dynamic Voltage and
Frequency Scaling (DVFS) for OMAP devices. DVFS is a technique that
uses the optimal operating frequency and voltage to allow a task to be
performed in the required amount of time.
OMAP processors have voltage domains whose voltage can be scaled to
various levels depending on which the operating frequencies of certain
devices belonging to the domain will also need to be scaled. This voltage
frequency tuple is known as Operating Performance Point (OPP). A device
can have multiple OPP's. Also a voltage domain could be shared between
multiple devices. Also there could be dependencies between various
voltage domains for maintaining system performance like VDD<X>
should be at voltage v1 when VDD<Y> is at voltage v2.

The design of this framework take into account all the above mentioned points.
To summarize the basic design of DVFS framework:-

1. Have device opp tables for each device whose operating frequency can be
   scaled. This is easy now due to the existance of hwmod layer which
   allow storing of device specific info. The device opp tables contain
   the opp pairs (frequency voltage tuples), the voltage domain pointer
   to which the device belongs to, the device specific set_rate and
   get_rate API's which will do the actual scaling of the device frequency
   and retrieve the current device frequency.
2. Introduce use counting on a per VDD basis. This is to take care multiple
   requests to scale a VDD. The VDD will be scaled to the maximum of the
   voltages requested.
3. Keep track of all scalable devices belonging to a particular voltage
   domain the voltage layer.
4. Generic API in the omap device layer which can be called by anybody
   to scale a device opp. This API will take in the device pointer and
   frequency to which the device needs to be scaled to. This API will
   then internally find out the voltage domain to which the device
   belongs to and the voltage to which the voltage domain needs to
   be put to for the device to be scaled to the new frequency from
   the device opp table. Then this API will call into the newly
   introduced API in voltage layer (as mentioned in 2) to see if
   there are other requests for the associated voltage domain to
   be at a voltage higher than the current chosen one. If not this
   API will go ahead and scale the voltage domain to the new voltage,
   run through the list of all scalable devices belonging to this
   voltage domain and scale them to the appropriate frequencies using
   the set_rate pointer in the device opp tables.
5. Handle inter VDD dependecies.

Work pending -
2. Add OMAP4 support.

Contributors to conceptualization of the design include
Benoit Cousson <b-cousson@ti.com>,
Kevin Hilman <khilman@deeprootsystems.com>,
Paul Wamsley <paul@pwsan.com>,
Vishwanath Sripathy <vishwanath.bs@ti.com>
Parthasarathy Basak <p-basak2@ti.com>
Anand Sawant <sawant@ti.com>

This patch series is primarily based of origin/pm-opp branch
of kevin's PM tree and due to it's dependency on the newly
introduced opp and voltage layer, and to test dvfs using
cpufreq layer the following are the dependent patches
to be applied in order.

	https://patchwork.kernel.org/patch/119544/
        https://patchwork.kernel.org/patch/117347/
        https://patchwork.kernel.org/patch/117348/
        https://patchwork.kernel.org/patch/117349/
        http://marc.info/?l=linux-omap&m=128162263809748&w=2
        https://patchwork.kernel.org/patch/119854/
        all 5 patches from origin/pm-cpufreq branch off Kevin's pm tree
        http://marc.info/?l=linux-omap&m=128170725127719&w=2
                                - all eight patches in this series
	http://marc.info/?l=linux-omap&m=128213020527909&w=2
				- all 10 patches in this series

This series has been tested on OMAP3430 SDP for mpu, iva and
core  DVFS through cpu freq framework.

Thara Gopinath (13):
  OMAP: Introduce a user list for each voltage domain instance in the
    voltage driver.
  OMAP: Introduce API in the OPP layer to find the opp entry
    corresponding to a voltage.
  OMAP: Introduce voltage domain information in the hwmod structures
  OMAP: Introduce API to return a device list associated with a voltage
    domain
  OMAP: Introduce device specific set rate and get rate in device opp
    structures.
  OMAP: Voltage layer changes to support DVFS.
  OMAP: Introduce dependent voltage domain support.
  OMAP: Introduce device set_rate and get_rate.
  OMAP: Disable smartreflex across DVFS
  OMAP3: Introduce custom set rate and get rate APIs for scalable
    devices
  OMAP3: Update cpufreq driver to use the new set_rate API
  OMAP3: Introduce voltage domain info in the hwmod structures.
  OMAP3: Add voltage dependency table for VDD1.

 arch/arm/mach-omap2/cpufreq34xx.c             |  104 ++++++++
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c    |    3 +
 arch/arm/mach-omap2/voltage.c                 |  313 +++++++++++++++++++++++++
 arch/arm/plat-omap/cpu-omap.c                 |    3 +-
 arch/arm/plat-omap/include/plat/omap_device.h |    3 +
 arch/arm/plat-omap/include/plat/omap_hwmod.h  |    5 +
 arch/arm/plat-omap/include/plat/opp.h         |   45 ++++-
 arch/arm/plat-omap/include/plat/voltage.h     |    4 +
 arch/arm/plat-omap/omap_device.c              |   74 ++++++
 arch/arm/plat-omap/opp.c                      |  159 +++++++++++++
 10 files changed, 711 insertions(+), 2 deletions(-)

-- 
1.7.1.GIT

[PATCH 01/13] OMAP: Introduce a user list for each voltage domain instance in the voltage driver.

[Thara Gopinath]

This patch introduces a user list of devices associated with each
voltage domain instance. The user list is implemented using plist
structure with priority node populated with the voltage values.
This patch also adds an API which will take in a device and
requested voltage as parameters, adds the info to the user list
and returns back the maximum voltage requested by all the user
devices. This can be used anytime to get the voltage that the
voltage domain instance can be transitioned into.

Signed-off-by: Thara Gopinath <thara@ti.com>
---
 arch/arm/mach-omap2/voltage.c             |   94 +++++++++++++++++++++++++++++
 arch/arm/plat-omap/include/plat/voltage.h |    2 +
 2 files changed, 96 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/voltage.c b/arch/arm/mach-omap2/voltage.c
index 6a07fe9..4624250 100644
--- a/arch/arm/mach-omap2/voltage.c
+++ b/arch/arm/mach-omap2/voltage.c
@@ -24,6 +24,9 @@
 #include <linux/clk.h>
 #include <linux/err.h>
 #include <linux/debugfs.h>
+#include <linux/spinlock.h>
+#include <linux/plist.h>
+#include <linux/slab.h>
 
 #include <plat/omap-pm.h>
 #include <plat/omap34xx.h>
@@ -95,6 +98,20 @@ struct vp_reg_val {
 };
 
 /**
+ * omap_vdd_user_list	- The per vdd user list
+ *
+ * @dev		: The device asking for the vdd to be set at a particular
+ *		  voltage
+ * @node	: The list head entry
+ * @volt	: The voltage requested by the device <dev>
+ */
+struct omap_vdd_user_list {
+	struct device *dev;
+	struct plist_node node;
+	u32 volt;
+};
+
+/**
  * omap_vdd_info - Per Voltage Domain info
  *
  * @volt_data		: voltage table having the distinct voltages supported
@@ -105,6 +122,9 @@ struct vp_reg_val {
  *			  vp registers
  * @volt_clk		: the clock associated with the vdd.
  * @opp_dev		: the 'struct device' associated with this vdd.
+ * @user_lock		: the lock to be used by the plist user_list
+ * @user_list		: the list head maintaining the various users
+ *			  of this vdd with the voltage requested by each user.
  * @volt_data_count	: Number of distinct voltages supported by this vdd.
  * @nominal_volt	: Nominal voltaged for this vdd.
  * cmdval_reg		: Voltage controller cmdval register.
@@ -117,6 +137,9 @@ struct omap_vdd_info{
 	struct clk *volt_clk;
 	struct device *opp_dev;
 	struct voltagedomain voltdm;
+	spinlock_t user_lock;
+	struct plist_head user_list;
+	struct mutex scaling_mutex;
 	int volt_data_count;
 	unsigned long nominal_volt;
 	u8 cmdval_reg;
@@ -785,11 +808,18 @@ static void __init vdd_data_configure(struct omap_vdd_info *vdd)
 	struct dentry *vdd_debug;
 	char name[16];
 #endif
+
 	if (cpu_is_omap34xx())
 		omap3_vdd_data_configure(vdd);
 	else if (cpu_is_omap44xx())
 		omap4_vdd_data_configure(vdd);
 
+	/* Init the plist */
+	spin_lock_init(&vdd->user_lock);
+	plist_head_init(&vdd->user_list, &vdd->user_lock);
+	/* Init the DVFS mutex */
+	mutex_init(&vdd->scaling_mutex);
+
 #ifdef CONFIG_PM_DEBUG
 	strcpy(name, "vdd_");
 	strcat(name, vdd->voltdm.name);
@@ -1142,6 +1172,70 @@ unsigned long omap_vp_get_curr_volt(struct voltagedomain *voltdm)
 }
 
 /**
+ * omap_voltage_add_userreq : API to keep track of various requests to
+ *			    scale the VDD and returns the best possible
+ *			    voltage the VDD can be put to.
+ * @volt_domain: pointer to the voltage domain.
+ * @dev : the device pointer.
+ * @volt : the voltage which is requested by the device.
+ *
+ * This API is to be called before the actual voltage scaling is
+ * done to determine what is the best possible voltage the VDD can
+ * be put to. This API adds the device <dev> in the user list of the
+ * vdd <volt_domain> with <volt> as the requested voltage. The user list
+ * is a plist with the priority element absolute voltage values.
+ * The API then finds the maximum of all the requested voltages for
+ * the VDD and returns it back through <volt> pointer itself.
+ * Returns error value in case of any errors.
+ */
+int omap_voltage_add_userreq(struct voltagedomain *voltdm, struct device *dev,
+		unsigned long *volt)
+{
+	struct omap_vdd_info *vdd;
+	struct omap_vdd_user_list *user;
+	struct plist_node *node;
+	int found = 0;
+
+	if (!voltdm || IS_ERR(voltdm)) {
+		pr_warning("%s: VDD specified does not exist!\n", __func__);
+		return -EINVAL;
+	}
+
+	vdd = container_of(voltdm, struct omap_vdd_info, voltdm);
+
+	mutex_lock(&vdd->scaling_mutex);
+
+	plist_for_each_entry(user, &vdd->user_list, node) {
+		if (user->dev == dev) {
+			found = 1;
+			break;
+		}
+	}
+
+	if (!found) {
+		user = kzalloc(sizeof(struct omap_vdd_user_list), GFP_KERNEL);
+		if (!user) {
+			pr_err("%s: Unable to creat a new user for vdd_%s\n",
+				__func__, voltdm->name);
+			mutex_unlock(&vdd->scaling_mutex);
+			return -ENOMEM;
+		}
+		user->dev = dev;
+	} else {
+		plist_del(&user->node, &vdd->user_list);
+	}
+
+	plist_node_init(&user->node, *volt);
+	plist_add(&user->node, &vdd->user_list);
+	node = plist_first(&vdd->user_list);
+	*volt = node->prio;
+
+	mutex_unlock(&vdd->scaling_mutex);
+
+	return 0;
+}
+
+/**
  * omap_vp_enable : API to enable a particular VP
  * @voltdm: pointer to the VDD whose VP is to be enabled.
  *
diff --git a/arch/arm/plat-omap/include/plat/voltage.h b/arch/arm/plat-omap/include/plat/voltage.h
index 3e8cf03..fffd086 100644
--- a/arch/arm/plat-omap/include/plat/voltage.h
+++ b/arch/arm/plat-omap/include/plat/voltage.h
@@ -144,6 +144,8 @@ struct omap_volt_data *omap_voltage_get_voltdata(struct voltagedomain *voltdm,
 		unsigned long volt);
 void omap_voltage_register_pmic(struct omap_volt_pmic_info *pmic_info);
 unsigned long omap_voltage_get_nom_volt(struct voltagedomain *voltdm);
+int omap_voltage_add_userreq(struct voltagedomain *voltdm, struct device *dev,
+		unsigned long *volt);
 #ifdef CONFIG_PM
 void omap_voltage_init_vc(struct omap_volt_vc_data *setup_vc);
 void omap_change_voltscale_method(int voltscale_method);
-- 
1.7.1.GIT

[PATCH 02/13] OMAP: Introduce API in the OPP layer to find the opp entry corresponding to a voltage.

[Thara Gopinath]

This patch adds an API in the opp layer to get the opp table entry
corresponding to the voltage passed as the parameter.

Signed-off-by: Thara Gopinath <thara@ti.com>
---
 arch/arm/plat-omap/include/plat/opp.h |    8 ++++++++
 arch/arm/plat-omap/opp.c              |   28 ++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-omap/include/plat/opp.h b/arch/arm/plat-omap/include/plat/opp.h
index 997b56e..0e580ed 100644
--- a/arch/arm/plat-omap/include/plat/opp.h
+++ b/arch/arm/plat-omap/include/plat/opp.h
@@ -76,6 +76,8 @@ struct omap_opp *opp_find_freq_floor(struct device *dev, unsigned long *freq);
 
 struct omap_opp *opp_find_freq_ceil(struct device *dev, unsigned long *freq);
 
+struct omap_opp *opp_find_voltage(struct device *dev, unsigned long volt);
+
 int opp_add(const struct omap_opp_def *opp_def);
 
 int opp_enable(struct omap_opp *opp);
@@ -119,6 +121,12 @@ static inline struct omap_opp *opp_find_freq_ceil(struct omap_opp *oppl,
 	return ERR_PTR(-EINVAL);
 }
 
+static inline struct omap_opp *opp_find_voltage(struct device *dev,
+						unsigned long volt)
+{
+	return ERR_PTR(-EINVAL);
+}
+
 static inline struct omap_opp *opp_add(struct omap_opp *oppl,
 				       const struct omap_opp_def *opp_def)
 {
diff --git a/arch/arm/plat-omap/opp.c b/arch/arm/plat-omap/opp.c
index 5a86bdd..a3dea82 100644
--- a/arch/arm/plat-omap/opp.c
+++ b/arch/arm/plat-omap/opp.c
@@ -290,6 +290,34 @@ struct omap_opp *opp_find_freq_floor(struct device *dev, unsigned long *freq)
 	return opp;
 }
 
+/**
+ * opp_find_voltage() - search for an exact voltage
+ * @dev:	device pointer associated with the opp type
+ * @volt:	voltage to search for
+ *
+ * Searches for exact 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.
+ */
+struct omap_opp *opp_find_voltage(struct device *dev, unsigned long volt)
+{
+	struct device_opp *dev_opp;
+	struct omap_opp *temp_opp, *opp = ERR_PTR(-ENODEV);
+
+	dev_opp = find_device_opp(dev);
+	if (IS_ERR(dev_opp))
+		return opp;
+
+	list_for_each_entry(temp_opp, &dev_opp->opp_list, node) {
+		if (temp_opp->enabled && temp_opp->u_volt == volt) {
+			opp = temp_opp;
+			break;
+		}
+	}
+
+	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)
-- 
1.7.1.GIT

[PATCH 03/13] OMAP: Introduce voltage domain information in the hwmod structures

[Thara Gopinath]

This patch extends the device hwmod structure to contain
info about the voltage domain to which the device belongs to.
This is needed to support a device based DVFS where the
device knows which voltage domain it belongs to.

Signed-off-by: Thara Gopinath <thara@ti.com>
---
 arch/arm/plat-omap/include/plat/omap_hwmod.h |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-omap/include/plat/omap_hwmod.h b/arch/arm/plat-omap/include/plat/omap_hwmod.h
index cab4a68..367b7ca 100644
--- a/arch/arm/plat-omap/include/plat/omap_hwmod.h
+++ b/arch/arm/plat-omap/include/plat/omap_hwmod.h
@@ -36,6 +36,7 @@
 #include <linux/list.h>
 #include <linux/ioport.h>
 #include <plat/cpu.h>
+#include <plat/voltage.h>
 
 struct omap_device;
 
@@ -415,6 +416,8 @@ struct omap_hwmod_class {
  * @main_clk: main clock: OMAP clock name
  * @_clk: pointer to the main struct clk (filled in at runtime)
  * @opt_clks: other device clocks that drivers can request (0..*)
+ * @vdd_name: voltage domain name
+ * @voltdm: pointer to voltage domain (filled in at runtime)
  * @masters: ptr to array of OCP ifs that this hwmod can initiate on
  * @slaves: ptr to array of OCP ifs that this hwmod can respond on
  * @dev_attr: arbitrary device attributes that can be passed to the driver
@@ -456,6 +459,8 @@ struct omap_hwmod {
 	const char			*main_clk;
 	struct clk			*_clk;
 	struct omap_hwmod_opt_clk	*opt_clks;
+	const char			*vdd_name;
+	struct voltagedomain		*voltdm;
 	struct omap_hwmod_ocp_if	**masters; /* connect to *_IA */
 	struct omap_hwmod_ocp_if	**slaves;  /* connect to *_TA */
 	void				*dev_attr;
-- 
1.7.1.GIT

[PATCH 04/13] OMAP: Introduce API to return a device list associated with a voltage domain

[Thara Gopinath]

This patch adds an API in the opp layer that
can be used by the voltage layer to get a list of all the
scalable devices belonging to a particular voltage domain.
This API is to be typically called only once by the voltage
layer per voltage domain instance and the device list should
be stored. This approach makes it easy during dvfs to scale
all the devices associated with a voltage domain and then
scale the voltage domain.

Signed-off-by: Thara Gopinath <thara@ti.com>
---
 arch/arm/plat-omap/include/plat/opp.h |    9 +++++++++
 arch/arm/plat-omap/opp.c              |   28 ++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-omap/include/plat/opp.h b/arch/arm/plat-omap/include/plat/opp.h
index 0e580ed..a4c1669 100644
--- a/arch/arm/plat-omap/include/plat/opp.h
+++ b/arch/arm/plat-omap/include/plat/opp.h
@@ -18,6 +18,7 @@
 #include <linux/cpufreq.h>
 
 #include <plat/common.h>
+#include <plat/voltage.h>
 
 /**
  * struct omap_opp_def - OMAP OPP Definition
@@ -86,6 +87,9 @@ int opp_disable(struct omap_opp *opp);
 
 void opp_init_cpufreq_table(struct device *dev,
 			    struct cpufreq_frequency_table **table);
+
+struct device **opp_init_voltage_params(struct voltagedomain *voltdm,
+					int *dev_count);
 #else
 static inline unsigned long opp_get_voltage(const struct omap_opp *opp)
 {
@@ -149,5 +153,10 @@ void opp_init_cpufreq_table(struct omap_opp *opps,
 {
 }
 
+static inline struct device **opp_init_voltage_params(
+			struct voltagedomain *voltdm, int *dev_count)
+{
+}
+
 #endif		/* CONFIG_PM */
 #endif		/* __ASM_ARM_OMAP_OPP_H */
diff --git a/arch/arm/plat-omap/opp.c b/arch/arm/plat-omap/opp.c
index a3dea82..72dd62a 100644
--- a/arch/arm/plat-omap/opp.c
+++ b/arch/arm/plat-omap/opp.c
@@ -502,3 +502,31 @@ void opp_init_cpufreq_table(struct device *dev,
 
 	*table = &freq_table[0];
 }
+
+struct device **opp_init_voltage_params(struct voltagedomain *voltdm,
+					int *dev_count)
+{
+	struct device_opp *dev_opp;
+	struct device **dev_list;
+	int count = 0, i = 0;
+
+	list_for_each_entry(dev_opp, &dev_opp_list, node) {
+		if (!dev_opp->oh->vdd_name)
+			continue;
+
+		if (!strcmp(dev_opp->oh->vdd_name, voltdm->name)) {
+			dev_opp->oh->voltdm = voltdm;
+			count++;
+		}
+	}
+
+	dev_list = kzalloc(sizeof(struct device *) * count, GFP_KERNEL);
+
+	list_for_each_entry(dev_opp, &dev_opp_list, node) {
+		if (dev_opp->oh->voltdm == voltdm)
+			dev_list[i++] = dev_opp->dev;
+	}
+
+	*dev_count = count;
+	return dev_list;
+}
-- 
1.7.1.GIT

[PATCH 05/13] OMAP: Introduce device specific set rate and get rate in device opp structures.

[Thara Gopinath]

This patch extends the device opp structure to contain
pointers to scale the operating rate of the
device and to retrieve the operating rate of the device.
This patch also adds the three new APIs in the opp layer
namely opp_set_rate that can be called to set a new operating
rate for a device, opp_get_rate that can be called to retrieve
the operating frequency for a device and opp_populate_rate_fns
to populte the device specific set_rate and get_rate API's.
The opp_set_rate and opp_get_rate does some routine error
checks and finally calls into the device specific set_rate
and get_rate APIs populated through opp_populate_rate_fns.

Signed-off-by: Thara Gopinath <thara@ti.com>
---
 arch/arm/plat-omap/include/plat/opp.h |   28 +++++++++-
 arch/arm/plat-omap/opp.c              |  103 +++++++++++++++++++++++++++++++++
 2 files changed, 130 insertions(+), 1 deletions(-)

diff --git a/arch/arm/plat-omap/include/plat/opp.h b/arch/arm/plat-omap/include/plat/opp.h
index a4c1669..604914d 100644
--- a/arch/arm/plat-omap/include/plat/opp.h
+++ b/arch/arm/plat-omap/include/plat/opp.h
@@ -16,6 +16,7 @@
 
 #include <linux/err.h>
 #include <linux/cpufreq.h>
+#include <linux/clk.h>
 
 #include <plat/common.h>
 #include <plat/voltage.h>
@@ -52,7 +53,7 @@ struct omap_opp_def {
  * To point at the end of a terminator of a list of OPPs,
  * use OMAP_OPP_DEF(NULL, 0, 0, 0)
  */
-#define OMAP_OPP_DEF(_hwmod_name, _enabled, _freq, _uv)	\
+#define OMAP_OPP_DEF(_hwmod_name, _enabled, _freq, _uv) \
 {						\
 	.hwmod_name	= _hwmod_name,		\
 	.enabled	= _enabled,		\
@@ -79,6 +80,14 @@ struct omap_opp *opp_find_freq_ceil(struct device *dev, unsigned long *freq);
 
 struct omap_opp *opp_find_voltage(struct device *dev, unsigned long volt);
 
+int opp_set_rate(struct device *dev, unsigned long freq);
+
+unsigned long opp_get_rate(struct device *dev);
+
+void opp_populate_rate_fns(struct device *dev,
+		int (*set_rate)(struct device *dev, unsigned long rate),
+		unsigned long (*get_rate) (struct device *dev));
+
 int opp_add(const struct omap_opp_def *opp_def);
 
 int opp_enable(struct omap_opp *opp);
@@ -131,6 +140,23 @@ static inline struct omap_opp *opp_find_voltage(struct device *dev,
 	return ERR_PTR(-EINVAL);
 }
 
+static inline int opp_set_rate(struct device *dev, unsigned long freq)
+{
+	return -EINVAL;
+}
+
+static inline unsigned long opp_get_rate(struct device *dev)
+{
+	return 0;
+}
+
+static inline void opp_populate_rate_fns(struct device *dev,
+		int (*set_rate)(struct device *dev, unsigned long rate)
+		unsigned long (*get_rate) (struct device *dev))
+{
+	return;
+}
+
 static inline struct omap_opp *opp_add(struct omap_opp *oppl,
 				       const struct omap_opp_def *opp_def)
 {
diff --git a/arch/arm/plat-omap/opp.c b/arch/arm/plat-omap/opp.c
index 72dd62a..3b5a581 100644
--- a/arch/arm/plat-omap/opp.c
+++ b/arch/arm/plat-omap/opp.c
@@ -21,6 +21,7 @@
 
 #include <plat/opp.h>
 #include <plat/omap_device.h>
+#include <plat/voltage.h>
 
 /**
  * struct omap_opp - OMAP OPP description structure
@@ -63,6 +64,9 @@ struct device_opp {
 	struct list_head opp_list;
 	u32 opp_count;
 	u32 enabled_opp_count;
+
+	int (*set_rate)(struct device *dev, unsigned long rate);
+	unsigned long (*get_rate) (struct device *dev);
 };
 
 static LIST_HEAD(dev_opp_list);
@@ -318,6 +322,105 @@ struct omap_opp *opp_find_voltage(struct device *dev, unsigned long volt)
 	return opp;
 }
 
+/**
+ * opp_set_rate() - Change the operating frequency of the device
+ * @dev:	device pointer associated with the opp type
+ * @freq:	new frequency at which the device is to be operated.
+ *
+ * This API calls into the custom specified set rate API mentioned
+ * in the device opp table to change the operating frequency of the
+ * device. Returns error values in case of no device opp table for the
+ * device or missing set_rate API in the device opp table.
+ */
+int opp_set_rate(struct device *dev, unsigned long freq)
+{
+	struct device_opp *dev_opp;
+
+	if (!dev) {
+		pr_err("%s: Invalid device\n", __func__);
+		return -EINVAL;
+	}
+
+	dev_opp = find_device_opp(dev);
+	if (IS_ERR(dev_opp)) {
+		dev_err(dev, "%s: No device opp table\n", __func__);
+		return -ENODEV;
+	}
+
+	if (!dev_opp->set_rate) {
+		dev_err(dev, "%s: No set_rate API for scaling opp\n",
+			__func__);
+		return -ENODATA;
+	}
+
+	return dev_opp->set_rate(dev, freq);
+}
+
+/**
+ * opp_get_rate() - Get the operating frequency of the device
+ * @dev:	device pointer associated with the opp type
+ *
+ * This API calls into the custom specified get rate API mentioned
+ * in the device opp table to retrieve the operating frequency of the
+ * device. Returns 0 in case of no device opp table for the
+ * device or missing get_rate API in the device opp table else
+ * returns the rate at which the device is operating.
+ */
+unsigned long opp_get_rate(struct device *dev)
+{
+	struct device_opp *dev_opp;
+
+	if (!dev) {
+		pr_err("%s: Invalid device\n", __func__);
+		return 0;
+	}
+
+	dev_opp = find_device_opp(dev);
+	if (IS_ERR(dev_opp)) {
+		dev_err(dev, "%s: No device opp table\n", __func__);
+		return 0;
+	}
+
+	if (!dev_opp->get_rate) {
+		dev_err(dev, "%s: No set_rate API for scaling opp\n",
+			__func__);
+		return 0;
+	}
+
+	return dev_opp->get_rate(dev);
+}
+
+/**
+ * opp_populate_rate_fns() - Populates the device opp tables with set_rate
+ *				and get_rate API's
+ * @dev:	device pointer whose device opp table is to be populated.
+ * @set_rate:	the set_rate API
+ * @get_rate:	the get_rate API
+ *
+ * This API populates the device opp table corresponding to device <dev>
+ * with the specified set_rate and get_rate APIs passed as parameters.
+ */
+void opp_populate_rate_fns(struct device *dev,
+		int (*set_rate)(struct device *dev, unsigned long rate),
+		unsigned long (*get_rate) (struct device *dev))
+{
+	struct device_opp *dev_opp;
+
+	if (!dev || !set_rate || !get_rate) {
+		pr_err("%s: Invalid device or parameters\n", __func__);
+		return;
+	}
+
+	dev_opp = find_device_opp(dev);
+	if (IS_ERR(dev_opp)) {
+		dev_err(dev, "%s: No device opp table\n", __func__);
+		return;
+	}
+
+	dev_opp->set_rate = set_rate;
+	dev_opp->get_rate = get_rate;
+}
+
 /* 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)
-- 
1.7.1.GIT

[PATCH 06/13] OMAP: Voltage layer changes to support DVFS.

[Thara Gopinath]

This patch introduces a list of scalable devices associated with
a particular voltage domain instance. This list is obtained from
the opp layer during init. This patch also introduces an API
to take in the voltage domain and the new voltage as parameter
and to scale all the scalable devices associated with the the
voltage domain to the rate corresponding to the new voltage and
scale the voltage domain to the new voltage.

Signed-off-by: Thara Gopinath <thara@ti.com>
---
 arch/arm/mach-omap2/voltage.c             |   71 +++++++++++++++++++++++++++++
 arch/arm/plat-omap/include/plat/voltage.h |    2 +
 2 files changed, 73 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/voltage.c b/arch/arm/mach-omap2/voltage.c
index 4624250..3332123 100644
--- a/arch/arm/mach-omap2/voltage.c
+++ b/arch/arm/mach-omap2/voltage.c
@@ -141,6 +141,8 @@ struct omap_vdd_info{
 	struct plist_head user_list;
 	struct mutex scaling_mutex;
 	int volt_data_count;
+	struct device **dev_list;
+	int dev_count;
 	unsigned long nominal_volt;
 	u8 cmdval_reg;
 	u8 vdd_sr_reg;
@@ -820,6 +822,9 @@ static void __init vdd_data_configure(struct omap_vdd_info *vdd)
 	/* Init the DVFS mutex */
 	mutex_init(&vdd->scaling_mutex);
 
+	/* Get the devices associated with this VDD */
+	vdd->dev_list = opp_init_voltage_params(&vdd->voltdm, &vdd->dev_count);
+
 #ifdef CONFIG_PM_DEBUG
 	strcpy(name, "vdd_");
 	strcat(name, vdd->voltdm.name);
@@ -1578,6 +1583,72 @@ struct voltagedomain *omap_voltage_domain_get(char *name)
 }
 
 /**
+ * omap_voltage_scale : API to scale the devices associated with a
+ *			voltage domain vdd voltage.
+ * @volt_domain : the voltage domain to be scaled
+ * @volt : the new voltage for the voltage domain
+ *
+ * This API runs through the list of devices associated with the
+ * voltage domain and scales the device rates to those corresponding
+ * to the new voltage of the voltage domain. This API also scales
+ * the voltage domain voltage to the new value. Returns 0 on success
+ * else the error value.
+ */
+int omap_voltage_scale(struct voltagedomain *voltdm, unsigned long volt)
+{
+	unsigned long curr_volt;
+	int is_volt_scaled = 0, i;
+	struct omap_vdd_info *vdd;
+
+	if (!voltdm || IS_ERR(voltdm)) {
+		pr_warning("%s: VDD specified does not exist!\n", __func__);
+		return -EINVAL;
+	}
+
+	vdd = container_of(voltdm, struct omap_vdd_info, voltdm);
+
+	mutex_lock(&vdd->scaling_mutex);
+
+	curr_volt = omap_voltage_get_nom_volt(voltdm);
+
+	if (curr_volt == volt) {
+		is_volt_scaled = 1;
+	} else if (curr_volt < volt) {
+		omap_voltage_scale_vdd(voltdm, volt);
+		is_volt_scaled = 1;
+	}
+
+	for (i = 0; i < vdd->dev_count; i++) {
+		struct omap_opp *opp;
+		unsigned long freq;
+
+		opp = opp_find_voltage(vdd->dev_list[i], volt);
+		if (IS_ERR(opp)) {
+			dev_err(vdd->dev_list[i], "%s: Unable to find OPP for"
+				"volt%ld\n", __func__, volt);
+			continue;
+		}
+
+		freq = opp_get_freq(opp);
+
+		if (freq == opp_get_rate(vdd->dev_list[i])) {
+			dev_warn(vdd->dev_list[i], "%s: Already at the"
+				"requested rate %ld\n", __func__, freq);
+			continue;
+		}
+
+		opp_set_rate(vdd->dev_list[i], freq);
+	}
+
+	if (!is_volt_scaled)
+		omap_voltage_scale_vdd(voltdm, volt);
+
+	mutex_unlock(&vdd->scaling_mutex);
+
+	return 0;
+}
+
+/**
  * omap_voltage_init : Volatage init API which does VP and VC init.
  */
 static int __init omap_voltage_init(void)
diff --git a/arch/arm/plat-omap/include/plat/voltage.h b/arch/arm/plat-omap/include/plat/voltage.h
index fffd086..4f8342a 100644
--- a/arch/arm/plat-omap/include/plat/voltage.h
+++ b/arch/arm/plat-omap/include/plat/voltage.h
@@ -146,6 +146,8 @@ void omap_voltage_register_pmic(struct omap_volt_pmic_info *pmic_info);
 unsigned long omap_voltage_get_nom_volt(struct voltagedomain *voltdm);
 int omap_voltage_add_userreq(struct voltagedomain *voltdm, struct device *dev,
 		unsigned long *volt);
+int omap_voltage_scale(struct voltagedomain *voltdm, unsigned long volt);
+
 #ifdef CONFIG_PM
 void omap_voltage_init_vc(struct omap_volt_vc_data *setup_vc);
 void omap_change_voltscale_method(int voltscale_method);
-- 
1.7.1.GIT

[PATCH 07/13] OMAP: Introduce dependent voltage domain support.

[Thara Gopinath]

There could be dependencies between various voltage domains for
maintaining system performance or hardware limitation reasons
like VDD<X> should be at voltage v1 when VDD<Y> is at voltage v2.
This patch introduce dependent vdd information structures in the
voltage layer which can be used to populate these dependencies
for a voltage domain. This patch also adds support to scale
the dependent vdd and the scalable devices belonging to it
during the scaling of a main vdd through omap_voltage_scale.

Signed-off-by: Thara Gopinath <thara@ti.com>
---
 arch/arm/mach-omap2/voltage.c |  122 +++++++++++++++++++++++++++++++++++++++++
 1 files changed, 122 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/voltage.c b/arch/arm/mach-omap2/voltage.c
index 3332123..1a46eb0 100644
--- a/arch/arm/mach-omap2/voltage.c
+++ b/arch/arm/mach-omap2/voltage.c
@@ -98,6 +98,36 @@ struct vp_reg_val {
 };
 
 /**
+ * omap_vdd_dep_volt - Table containing the parent vdd voltage and the
+ *			dependent vdd voltage corresponding to it.
+ *
+ * @main_vdd_volt	: The main vdd voltage
+ * @dep_vdd_volt	: The voltage at which the dependent vdd should be
+ *			  when the main vdd is at <main_vdd_volt> voltage
+ */
+struct omap_vdd_dep_volt {
+	u32 main_vdd_volt;
+	u32 dep_vdd_volt;
+};
+
+/**
+ * omap_vdd_dep_info - Dependent vdd info
+ *
+ * @name		: Dependent vdd name
+ * @voltdm		: Dependent vdd pointer
+ * @dep_table		: Table containing the dependent vdd voltage
+ *			  corresponding to every main vdd voltage.
+ * @cur_dep_volt	: The voltage to which dependent vdd should be put
+ *			  to for the current main vdd voltage.
+ */
+struct omap_vdd_dep_info{
+	char *name;
+	struct voltagedomain *voltdm;
+	struct omap_vdd_dep_volt *dep_table;
+	unsigned long cur_dep_volt;
+};
+
+/**
  * omap_vdd_user_list	- The per vdd user list
  *
  * @dev		: The device asking for the vdd to be set at a particular
@@ -137,10 +167,12 @@ struct omap_vdd_info{
 	struct clk *volt_clk;
 	struct device *opp_dev;
 	struct voltagedomain voltdm;
+	struct omap_vdd_dep_info *dep_vdd_info;
 	spinlock_t user_lock;
 	struct plist_head user_list;
 	struct mutex scaling_mutex;
 	int volt_data_count;
+	int nr_dep_vdd;
 	struct device **dev_list;
 	int dev_count;
 	unsigned long nominal_volt;
@@ -1114,6 +1146,80 @@ static int vp_forceupdate_scale_voltage(struct omap_vdd_info *vdd,
 	return 0;
 }
 
+static int calc_dep_vdd_volt(struct device *dev,
+		struct omap_vdd_info *main_vdd, unsigned long main_volt)
+{
+	struct omap_vdd_dep_info *dep_vdds;
+	int i, ret = 0;
+
+	if (!main_vdd->dep_vdd_info) {
+		pr_debug("%s: No dependent VDD's for vdd_%s\n",
+			__func__, main_vdd->voltdm.name);
+		return 0;
+	}
+
+	dep_vdds = main_vdd->dep_vdd_info;
+
+	for (i = 0; i < main_vdd->nr_dep_vdd; i++) {
+		struct omap_vdd_dep_volt *volt_table = dep_vdds[i].dep_table;
+		int nr_volt = 0;
+		unsigned long dep_volt = 0, act_volt = 0;
+
+		while (volt_table[nr_volt].main_vdd_volt != 0) {
+			if (volt_table[nr_volt].main_vdd_volt == main_volt) {
+				dep_volt = volt_table[nr_volt].dep_vdd_volt;
+				break;
+			}
+			nr_volt++;
+		}
+		if (!dep_volt) {
+			pr_warning("%s: Not able to find a matching volt for"
+				"vdd_%s corresponding to vdd_%s %ld volt\n",
+				__func__, dep_vdds[i].name,
+				main_vdd->voltdm.name, main_volt);
+			ret = -EINVAL;
+			continue;
+		}
+
+		if (!dep_vdds[i].voltdm)
+			dep_vdds[i].voltdm =
+				omap_voltage_domain_get(dep_vdds[i].name);
+
+		act_volt = dep_volt;
+
+		/* See if dep_volt is possible for the vdd*/
+		ret = omap_voltage_add_userreq(dep_vdds[i].voltdm, dev,
+				&act_volt);
+
+		/*
+		 * Currently we do not bother if the dep volt and act volt are
+		 * different. We could add a check if needed.
+		 */
+		dep_vdds[i].cur_dep_volt = act_volt;
+	}
+
+	return ret;
+}
+
+static int scale_dep_vdd(struct omap_vdd_info *main_vdd)
+{
+	struct omap_vdd_dep_info *dep_vdds;
+	int i;
+
+	if (!main_vdd->dep_vdd_info) {
+		pr_debug("%s: No dependent VDD's for vdd_%s\n",
+			__func__, main_vdd->voltdm.name);
+		return 0;
+	}
+
+	dep_vdds = main_vdd->dep_vdd_info;
+
+	for (i = 0; i < main_vdd->nr_dep_vdd; i++)
+		omap_voltage_scale(dep_vdds[i].voltdm,
+				dep_vdds[i].cur_dep_volt);
+	return 0;
+}
+
 /* Public functions */
 /**
  * omap_voltage_get_nom_volt : Gets the current non-auto-compensated voltage
@@ -1599,6 +1705,8 @@ int omap_voltage_scale(struct voltagedomain *voltdm, unsigned long volt)
 	unsigned long curr_volt;
 	int is_volt_scaled = 0, i;
 	struct omap_vdd_info *vdd;
+	struct plist_node *node;
+	struct omap_vdd_user_list *user;
 
 	if (!voltdm || IS_ERR(voltdm)) {
 		pr_warning("%s: VDD specified does not exist!\n", __func__);
@@ -1611,6 +1719,17 @@ int omap_voltage_scale(struct voltagedomain *voltdm, unsigned long volt)
 
 	curr_volt = omap_voltage_get_nom_volt(voltdm);
 
+	/* Find the device requesting the voltage scaling */
+	node = plist_first(&vdd->user_list);
+	user = container_of(node, struct omap_vdd_user_list, node);
+
+	/* calculate the voltages for dependent vdd's */
+	if (calc_dep_vdd_volt(user->dev, vdd, volt)) {
+		pr_warning("%s: Error in calculating dependent vdd voltages"
+			"for vdd_%s\n", __func__, voltdm->name);
+		return -EINVAL;
+	}
+
 	if (curr_volt == volt) {
 		is_volt_scaled = 1;
 	} else if (curr_volt < volt) {
@@ -1645,6 +1764,9 @@ int omap_voltage_scale(struct voltagedomain *voltdm, unsigned long volt)
 
 	mutex_unlock(&vdd->scaling_mutex);
 
+	/* Scale dependent vdds */
+	scale_dep_vdd(vdd);
+
 	return 0;
 }
 
-- 
1.7.1.GIT

[PATCH 08/13] OMAP: Introduce device set_rate and get_rate.

[Thara Gopinath]

This patch adds omap_device_set_rate and omap_device_get_rate
API's which can be used to generic device rate scaling.

Signed-off-by: Thara Gopinath <thara@ti.com>
---
 arch/arm/plat-omap/include/plat/omap_device.h |    3 +
 arch/arm/plat-omap/omap_device.c              |   74 +++++++++++++++++++++++++
 2 files changed, 77 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-omap/include/plat/omap_device.h b/arch/arm/plat-omap/include/plat/omap_device.h
index 25cd9ac..2ebf2e2 100644
--- a/arch/arm/plat-omap/include/plat/omap_device.h
+++ b/arch/arm/plat-omap/include/plat/omap_device.h
@@ -116,6 +116,9 @@ int omap_device_enable_hwmods(struct omap_device *od);
 int omap_device_disable_clocks(struct omap_device *od);
 int omap_device_enable_clocks(struct omap_device *od);
 
+int omap_device_set_rate(struct device *req_dev, struct device *dev,
+			 unsigned long rate);
+unsigned long omap_device_get_rate(struct device *dev);
 
 /*
  * Entries should be kept in latency order ascending
diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
index d2b1609..3c3ba43 100644
--- a/arch/arm/plat-omap/omap_device.c
+++ b/arch/arm/plat-omap/omap_device.c
@@ -85,6 +85,8 @@
 
 #include <plat/omap_device.h>
 #include <plat/omap_hwmod.h>
+#include <plat/opp.h>
+#include <plat/voltage.h>
 
 /* These parameters are passed to _omap_device_{de,}activate() */
 #define USE_WAKEUP_LAT			0
@@ -757,3 +759,75 @@ int omap_device_enable_clocks(struct omap_device *od)
 	/* XXX pass along return value here? */
 	return 0;
 }
+
+/**
+ * omap_device_set_rate - Set a new rate at which the device is to operate
+ * @req_dev : pointer to the device requesting the scaling.
+ * @dev : pointer to the device that is to be scaled
+ * @rate : the rnew rate for the device.
+ *
+ * This API gets the device opp table associated with this device and
+ * tries putting the device to the requested rate and the voltage domain
+ * associated with the device to the voltage corresponding to the
+ * requested rate. Since multiple devices can be assocciated with a
+ * voltage domain this API finds out the possible voltage the
+ * voltage domain can enter and then decides on the final device
+ * rate. Return 0 on success else the error value
+ */
+int omap_device_set_rate(struct device *req_dev, struct device *dev,
+			unsigned long rate)
+{
+	struct omap_opp *opp;
+	unsigned long volt, freq;
+	struct voltagedomain *voltdm;
+	struct platform_device *pdev;
+	struct omap_device *od;
+	int ret;
+
+	pdev = container_of(dev, struct platform_device, dev);
+	od = _find_by_pdev(pdev);
+
+	/* Get the possible rate from the opp layer */
+	freq = rate;
+	opp = opp_find_freq_ceil(dev, &freq);
+	if (IS_ERR(opp)) {
+		dev_err(dev, "%s: Unable to find OPP for freq%ld\n",
+			__func__, rate);
+		return -ENODEV;
+	}
+	if (unlikely(freq != rate))
+		dev_warn(dev, "%s: Available freq %ld != dpll freq %ld.\n",
+			__func__, freq, rate);
+
+	/* Get the voltage corresponding to the requested frequency */
+	volt = opp_get_voltage(opp);
+
+	/*
+	 * Call into the voltage layer to get the final voltage possible
+	 * for the voltage domain associated with the device.
+	 */
+	voltdm = od->hwmods[0]->voltdm;
+	ret = omap_voltage_add_userreq(voltdm, req_dev, &volt);
+	if (ret) {
+		dev_err(dev, "%s: Unable to get the final volt for scaling\n",
+			__func__);
+		return ret;
+	}
+
+	/* Do the actual scaling */
+	return omap_voltage_scale(voltdm, volt);
+}
+EXPORT_SYMBOL(omap_device_set_rate);
+
+/**
+ * omap_device_get_rate - Gets the current operating rate of the device
+ * @dev - the device pointer
+ *
+ * This API returns the current operating rate of the device on success.
+ * Else returns the error value.
+ */
+unsigned long omap_device_get_rate(struct device *dev)
+{
+	return opp_get_rate(dev);
+}
+EXPORT_SYMBOL(omap_device_get_rate);
-- 
1.7.1.GIT

[PATCH 09/13] OMAP: Disable smartreflex across DVFS

[Thara Gopinath]

This patch disables smartreflex for a particular voltage
domain when the the voltage domain and the devices belonging
to it is being scaled and re-enables it back once the scaling
is done.

Signed-off-by: Thara Gopinath <thara@ti.com>
---
 arch/arm/mach-omap2/voltage.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/voltage.c b/arch/arm/mach-omap2/voltage.c
index 1a46eb0..2fd644a 100644
--- a/arch/arm/mach-omap2/voltage.c
+++ b/arch/arm/mach-omap2/voltage.c
@@ -35,6 +35,7 @@
 #include <plat/clock.h>
 #include <plat/common.h>
 #include <plat/voltage.h>
+#include <plat/smartreflex.h>
 
 #include "prm-regbits-34xx.h"
 #include "prm44xx.h"
@@ -1730,6 +1731,9 @@ int omap_voltage_scale(struct voltagedomain *voltdm, unsigned long volt)
 		return -EINVAL;
 	}
 
+	/* Disable smartreflex module across voltage and frequency scaling */
+	omap_smartreflex_disable(voltdm);
+
 	if (curr_volt == volt) {
 		is_volt_scaled = 1;
 	} else if (curr_volt < volt) {
@@ -1764,6 +1768,9 @@ int omap_voltage_scale(struct voltagedomain *voltdm, unsigned long volt)
 
 	mutex_unlock(&vdd->scaling_mutex);
 
+	/* Enable Smartreflex module */
+	omap_smartreflex_enable(voltdm);
+
 	/* Scale dependent vdds */
 	scale_dep_vdd(vdd);
 
-- 
1.7.1.GIT

[PATCH 10/13] OMAP3: Introduce custom set rate and get rate APIs for scalable devices

[Thara Gopinath]

This patch also introduces omap3_mpu_set_rate, omap3_iva_set_rate,
omap3_l3_set_rate, omap3_mpu_get_rate, omap3_iva_get_rate,
omap3_l3_get_rate as device specific set rate and get rate
APIs for OMAP3 mpu, iva and l3_main devices. This patch also
calls into opp_populate_rate_fns during system init to register
various set_rate and get_rate APIs with the generic opp
framework.

Signed-off-by: Thara Gopinath <thara@ti.com>
---
 arch/arm/mach-omap2/cpufreq34xx.c |  104 +++++++++++++++++++++++++++++++++++++
 1 files changed, 104 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/cpufreq34xx.c b/arch/arm/mach-omap2/cpufreq34xx.c
index 195b5bc..8b9ef72 100644
--- a/arch/arm/mach-omap2/cpufreq34xx.c
+++ b/arch/arm/mach-omap2/cpufreq34xx.c
@@ -24,8 +24,22 @@
 
 #include <plat/opp.h>
 #include <plat/cpu.h>
+#include <plat/clock.h>
+
+#include "cm-regbits-34xx.h"
+#include "prm.h"
 #include "omap3-opp.h"
 
+static int omap3_mpu_set_rate(struct device *dev, unsigned long rate);
+static int omap3_iva_set_rate(struct device *dev, unsigned long rate);
+static int omap3_l3_set_rate(struct device *dev, unsigned long rate);
+
+static unsigned long omap3_mpu_get_rate(struct device *dev);
+static unsigned long omap3_iva_get_rate(struct device *dev);
+static unsigned long omap3_l3_get_rate(struct device *dev);
+
+struct clk *dpll1_clk, *dpll2_clk, *dpll3_clk;
+
 static struct omap_opp_def __initdata omap34xx_opp_def_list[] = {
 	/* MPU OPP1 */
 	OMAP_OPP_DEF("mpu", true, 125000000, 975000),
@@ -92,12 +106,82 @@ static struct omap_opp_def __initdata omap36xx_opp_def_list[] = {
 };
 static u32 omap36xx_opp_def_size = ARRAY_SIZE(omap36xx_opp_def_list);
 
+
+static int omap3_mpu_set_rate(struct device *dev, unsigned long rate)
+{
+	unsigned long cur_rate = omap3_mpu_get_rate(dev);
+	int ret;
+
+#ifdef CONFIG_CPU_FREQ
+	struct cpufreq_freqs freqs_notify;
+
+	freqs_notify.old = cur_rate / 1000;
+	freqs_notify.new = rate / 1000;
+	freqs_notify.cpu = 0;
+	/* Send pre notification to CPUFreq */
+	cpufreq_notify_transition(&freqs_notify, CPUFREQ_PRECHANGE);
+#endif
+	ret = clk_set_rate(dpll1_clk, rate);
+	if (ret) {
+		dev_warn(dev, "%s: Unable to set rate to %ld\n",
+			__func__, rate);
+		return ret;
+	}
+
+#ifdef CONFIG_CPU_FREQ
+	/* Send a post notification to CPUFreq */
+	cpufreq_notify_transition(&freqs_notify, CPUFREQ_POSTCHANGE);
+#endif
+
+#ifndef CONFIG_CPU_FREQ
+	/*Update loops_per_jiffy if processor speed is being changed*/
+	loops_per_jiffy = compute_lpj(loops_per_jiffy,
+			cur_rate / 1000, rate / 1000);
+#endif
+	return 0;
+}
+
+static unsigned long omap3_mpu_get_rate(struct device *dev)
+{
+	return dpll1_clk->rate;
+}
+
+static int omap3_iva_set_rate(struct device *dev, unsigned long rate)
+{
+	return clk_set_rate(dpll2_clk, rate);
+}
+
+static unsigned long omap3_iva_get_rate(struct device *dev)
+{
+	return dpll2_clk->rate;
+}
+
+static int omap3_l3_set_rate(struct device *dev, unsigned long rate)
+{
+	int l3_div;
+
+	l3_div = cm_read_mod_reg(CORE_MOD, CM_CLKSEL) &
+			OMAP3430_CLKSEL_L3_MASK;
+
+	return clk_set_rate(dpll3_clk, rate * l3_div);
+}
+
+static unsigned long omap3_l3_get_rate(struct device *dev)
+{
+	int l3_div;
+
+	l3_div = cm_read_mod_reg(CORE_MOD, CM_CLKSEL) &
+			OMAP3430_CLKSEL_L3_MASK;
+	return dpll3_clk->rate / l3_div;
+}
+
 /* Temp variable to allow multiple calls */
 static u8 __initdata omap3_table_init;
 
 int __init omap3_pm_init_opp_table(void)
 {
 	struct omap_opp_def *opp_def, *omap3_opp_def_list;
+	struct device *dev;
 	u32 omap3_opp_def_size;
 	int i, r;
 
@@ -122,6 +206,26 @@ int __init omap3_pm_init_opp_table(void)
 			       opp_def->freq, opp_def->hwmod_name);
 	}
 
+	dpll1_clk = clk_get(NULL, "dpll1_ck");
+	dpll2_clk = clk_get(NULL, "dpll2_ck");
+	dpll3_clk = clk_get(NULL, "dpll3_m2_ck");
+
+	/* Populate the set rate and get rate for mpu, iva and l3 device */
+	dev = omap2_get_mpuss_device();
+	if (dev)
+		opp_populate_rate_fns(dev, omap3_mpu_set_rate,
+				omap3_mpu_get_rate);
+
+	dev = omap2_get_iva_device();
+	if (dev)
+		opp_populate_rate_fns(dev, omap3_iva_set_rate,
+				omap3_iva_get_rate);
+
+	dev = omap2_get_l3_device();
+	if (dev)
+		opp_populate_rate_fns(dev, omap3_l3_set_rate,
+				omap3_l3_get_rate);
+
 	return 0;
 }
 
-- 
1.7.1.GIT

[PATCH 11/13] OMAP3: Update cpufreq driver to use the new set_rate API

[Thara Gopinath]

This patch updates the cpufreq driver to use the device
set rate API to scale the mpu frequency for OMAP3.

Signed-off-by: Thara Gopinath <thara@ti.com>
---
 arch/arm/plat-omap/cpu-omap.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/arch/arm/plat-omap/cpu-omap.c b/arch/arm/plat-omap/cpu-omap.c
index df08829..9232246 100644
--- a/arch/arm/plat-omap/cpu-omap.c
+++ b/arch/arm/plat-omap/cpu-omap.c
@@ -29,6 +29,7 @@
 #include <mach/hardware.h>
 #include <plat/clock.h>
 #include <asm/system.h>
+#include <plat/omap_device.h>
 
 #if defined(CONFIG_ARCH_OMAP3) && !defined(CONFIG_OMAP_PM_NONE)
 #include <plat/omap-pm.h>
@@ -117,7 +118,7 @@ static int omap_target(struct cpufreq_policy *policy,
 #elif defined(CONFIG_ARCH_OMAP3) && !defined(CONFIG_OMAP_PM_NONE)
 	freq = target_freq * 1000;
 	if (opp_find_freq_ceil(mpu_dev, &freq))
-		omap_pm_cpu_set_freq(freq);
+		omap_device_set_rate(mpu_dev, mpu_dev, freq);
 #endif
 	return ret;
 }
-- 
1.7.1.GIT

[PATCH 12/13] OMAP3: Introduce voltage domain info in the hwmod structures.

[Thara Gopinath]

This patch adds voltage domain info in the relevant
device hwmod structures so as to enable OMAP3 DVFS
support.

Signed-off-by: Thara Gopinath <thara@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
index c9f0948..a6a67ed 100644
--- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
@@ -77,6 +77,7 @@ static struct omap_hwmod_ocp_if *omap3xxx_l3_main_masters[] = {
 static struct omap_hwmod omap3xxx_l3_main_hwmod = {
 	.name		= "l3_main",
 	.class		= &l3_hwmod_class,
+	.vdd_name	= "core",
 	.masters	= omap3xxx_l3_main_masters,
 	.masters_cnt	= ARRAY_SIZE(omap3xxx_l3_main_masters),
 	.slaves		= omap3xxx_l3_main_slaves,
@@ -206,6 +207,7 @@ static struct omap_hwmod omap3xxx_mpu_hwmod = {
 	.name		= "mpu",
 	.class		= &mpu_hwmod_class,
 	.main_clk	= "arm_fck",
+	.vdd_name	= "mpu",
 	.masters	= omap3xxx_mpu_masters,
 	.masters_cnt	= ARRAY_SIZE(omap3xxx_mpu_masters),
 	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
@@ -234,6 +236,7 @@ static struct omap_hwmod_ocp_if *omap3xxx_iva_masters[] = {
 static struct omap_hwmod omap3xxx_iva_hwmod = {
 	.name		= "iva",
 	.class		= &iva_hwmod_class,
+	.vdd_name	= "mpu",
 	.masters	= omap3xxx_iva_masters,
 	.masters_cnt	= ARRAY_SIZE(omap3xxx_iva_masters),
 	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430)
-- 
1.7.1.GIT

[PATCH 13/13] OMAP3: Add voltage dependency table for VDD1.

[Thara Gopinath]

In OMAP3, for perfomrance reasons when VDD1 is at voltage above
1.075V, VDD2 should be at 1.15V for perfomrance reasons. This
patch introduce this cross VDD dependency for OMAP3 VDD1.

Signed-off-by: Thara Gopinath <thara@ti.com>
---
 arch/arm/mach-omap2/voltage.c |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/voltage.c b/arch/arm/mach-omap2/voltage.c
index 2fd644a..3c990d0 100644
--- a/arch/arm/mach-omap2/voltage.c
+++ b/arch/arm/mach-omap2/voltage.c
@@ -348,6 +348,23 @@ static struct omap_volt_data omap44xx_vdd_core_volt_data[] = {
 	{.volt_nominal = 1100000, .sr_errminlimit = 0xF9, .vp_errgain = 0x16},
 };
 
+/* OMAP 3430 MPU Core VDD dependency table */
+static struct omap_vdd_dep_volt omap34xx_vdd1_vdd2_data[] = {
+	{.main_vdd_volt = 975000, .dep_vdd_volt = 1050000},
+	{.main_vdd_volt = 1075000, .dep_vdd_volt = 1050000},
+	{.main_vdd_volt = 1200000, .dep_vdd_volt = 1150000},
+	{.main_vdd_volt = 1270000, .dep_vdd_volt = 1150000},
+	{.main_vdd_volt = 1350000, .dep_vdd_volt = 1150000},
+	{.main_vdd_volt = 0, .dep_vdd_volt = 0},
+};
+
+static struct omap_vdd_dep_info omap34xx_vdd1_dep_info[] = {
+	{
+		.name	= "core",
+		.dep_table = omap34xx_vdd1_vdd2_data,
+	},
+};
+
 /* By default VPFORCEUPDATE is the chosen method of voltage scaling */
 static bool voltscale_vpforceupdate = true;
 
@@ -523,6 +540,8 @@ static void __init omap3_vdd_data_configure(struct omap_vdd_info *vdd)
 			vdd->volt_data = omap34xx_vdd1_volt_data;
 			vdd->volt_data_count =
 					ARRAY_SIZE(omap34xx_vdd1_volt_data);
+			vdd->dep_vdd_info = omap34xx_vdd1_dep_info;
+			vdd->nr_dep_vdd = ARRAY_SIZE(omap34xx_vdd1_dep_info);
 		}
 		vdd->volt_clk = clk_get(NULL, "dpll1_ck");
 		vdd->opp_dev = omap2_get_mpuss_device();
-- 
1.7.1.GIT

Re: [PATCH 01/13] OMAP: Introduce a user list for each voltage domain instance in the voltage driver.

[Kevin Hilman]

Thara Gopinath <thara@ti.com> writes:

> This patch introduces a user list of devices associated with each
> voltage domain instance. The user list is implemented using plist
> structure with priority node populated with the voltage values.
> This patch also adds an API which will take in a device and
> requested voltage as parameters, adds the info to the user list
> and returns back the maximum voltage requested by all the user
> devices. This can be used anytime to get the voltage that the
> voltage domain instance can be transitioned into.
>
> Signed-off-by: Thara Gopinath <thara@ti.com>
> ---
>  arch/arm/mach-omap2/voltage.c             |   94 +++++++++++++++++++++++++++++
>  arch/arm/plat-omap/include/plat/voltage.h |    2 +
>  2 files changed, 96 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/voltage.c b/arch/arm/mach-omap2/voltage.c
> index 6a07fe9..4624250 100644
> --- a/arch/arm/mach-omap2/voltage.c
> +++ b/arch/arm/mach-omap2/voltage.c
> @@ -24,6 +24,9 @@
>  #include <linux/clk.h>
>  #include <linux/err.h>
>  #include <linux/debugfs.h>
> +#include <linux/spinlock.h>
> +#include <linux/plist.h>
> +#include <linux/slab.h>
>  
>  #include <plat/omap-pm.h>
>  #include <plat/omap34xx.h>
> @@ -95,6 +98,20 @@ struct vp_reg_val {
>  };
>  
>  /**
> + * omap_vdd_user_list	- The per vdd user list
> + *
> + * @dev		: The device asking for the vdd to be set at a particular
> + *		  voltage
> + * @node	: The list head entry
> + * @volt	: The voltage requested by the device <dev>
> + */
> +struct omap_vdd_user_list {
> +	struct device *dev;
> +	struct plist_node node;
> +	u32 volt;
> +};
> +
> +/**
>   * omap_vdd_info - Per Voltage Domain info
>   *
>   * @volt_data		: voltage table having the distinct voltages supported
> @@ -105,6 +122,9 @@ struct vp_reg_val {
>   *			  vp registers
>   * @volt_clk		: the clock associated with the vdd.
>   * @opp_dev		: the 'struct device' associated with this vdd.
> + * @user_lock		: the lock to be used by the plist user_list
> + * @user_list		: the list head maintaining the various users
> + *			  of this vdd with the voltage requested by each user.
>   * @volt_data_count	: Number of distinct voltages supported by this vdd.
>   * @nominal_volt	: Nominal voltaged for this vdd.
>   * cmdval_reg		: Voltage controller cmdval register.
> @@ -117,6 +137,9 @@ struct omap_vdd_info{
>  	struct clk *volt_clk;
>  	struct device *opp_dev;
>  	struct voltagedomain voltdm;
> +	spinlock_t user_lock;
> +	struct plist_head user_list;
> +	struct mutex scaling_mutex;
>  	int volt_data_count;
>  	unsigned long nominal_volt;
>  	u8 cmdval_reg;
> @@ -785,11 +808,18 @@ static void __init vdd_data_configure(struct omap_vdd_info *vdd)
>  	struct dentry *vdd_debug;
>  	char name[16];
>  #endif
> +
>  	if (cpu_is_omap34xx())
>  		omap3_vdd_data_configure(vdd);
>  	else if (cpu_is_omap44xx())
>  		omap4_vdd_data_configure(vdd);
>  
> +	/* Init the plist */
> +	spin_lock_init(&vdd->user_lock);
> +	plist_head_init(&vdd->user_list, &vdd->user_lock);
> +	/* Init the DVFS mutex */
> +	mutex_init(&vdd->scaling_mutex);
> +
>  #ifdef CONFIG_PM_DEBUG
>  	strcpy(name, "vdd_");
>  	strcat(name, vdd->voltdm.name);
> @@ -1142,6 +1172,70 @@ unsigned long omap_vp_get_curr_volt(struct voltagedomain *voltdm)
>  }
>  
>  /**
> + * omap_voltage_add_userreq : API to keep track of various requests to
> + *			    scale the VDD and returns the best possible
> + *			    voltage the VDD can be put to.
> + * @volt_domain: pointer to the voltage domain.
> + * @dev : the device pointer.
> + * @volt : the voltage which is requested by the device.
> + *
> + * This API is to be called before the actual voltage scaling is
> + * done to determine what is the best possible voltage the VDD can
> + * be put to. This API adds the device <dev> in the user list of the
> + * vdd <volt_domain> with <volt> as the requested voltage. The user list
> + * is a plist with the priority element absolute voltage values.
> + * The API then finds the maximum of all the requested voltages for
> + * the VDD and returns it back through <volt> pointer itself.
> + * Returns error value in case of any errors.
> + */
> +int omap_voltage_add_userreq(struct voltagedomain *voltdm, struct device *dev,
> +		unsigned long *volt)

How about just omap_voltage_add_request()

Also, do we need both voltdm and dev?  With your other patches, can't we
look up the voltm based on dev?  Or, is there a need for a device to add
a request in a voltage domain other than the one to which it belongs?

Also, how does one remove a voltage request?

> +{
> +	struct omap_vdd_info *vdd;
> +	struct omap_vdd_user_list *user;
> +	struct plist_node *node;
> +	int found = 0;
> +
> +	if (!voltdm || IS_ERR(voltdm)) {
> +		pr_warning("%s: VDD specified does not exist!\n", __func__);
> +		return -EINVAL;
> +	}
> +
> +	vdd = container_of(voltdm, struct omap_vdd_info, voltdm);
> +
> +	mutex_lock(&vdd->scaling_mutex);
> +
> +	plist_for_each_entry(user, &vdd->user_list, node) {
> +		if (user->dev == dev) {
> +			found = 1;
> +			break;
> +		}
> +	}

Minor: I'm not crazy about the 'found' flag.  IMO, readability is
improved if you init 'user = NULL' above, use a tmp pointer to
walk the list, and rather than 'found = 1', do 'user = tmp_user'
when you find the match.

> +
> +	if (!found) {

and here, do if (!user)

> +		user = kzalloc(sizeof(struct omap_vdd_user_list), GFP_KERNEL);
> +		if (!user) {
> +			pr_err("%s: Unable to creat a new user for vdd_%s\n",
> +				__func__, voltdm->name);
> +			mutex_unlock(&vdd->scaling_mutex);
> +			return -ENOMEM;
> +		}
> +		user->dev = dev;
> +	} else {
> +		plist_del(&user->node, &vdd->user_list);

hmm... if we get here, we didn't find a match, so 'user' points to the
last element in the list (which is the highest voltage request, I
guess), or even NULL if the list is empty.  Then, this node is deleted.

-ECONFUSED

> +	}
> +
> +	plist_node_init(&user->node, *volt);
> +	plist_add(&user->node, &vdd->user_list);
> +	node = plist_first(&vdd->user_list);
> +	*volt = node->prio;
> +
> +	mutex_unlock(&vdd->scaling_mutex);
> +
> +	return 0;
> +}

Can't think of a use-case now (cuz it's Friday), but would we ever want
to add multiple requests per device?

The current approch will only track one request per device.

Kevin

> +/**
>   * omap_vp_enable : API to enable a particular VP
>   * @voltdm: pointer to the VDD whose VP is to be enabled.
>   *
> diff --git a/arch/arm/plat-omap/include/plat/voltage.h b/arch/arm/plat-omap/include/plat/voltage.h
> index 3e8cf03..fffd086 100644
> --- a/arch/arm/plat-omap/include/plat/voltage.h
> +++ b/arch/arm/plat-omap/include/plat/voltage.h
> @@ -144,6 +144,8 @@ struct omap_volt_data *omap_voltage_get_voltdata(struct voltagedomain *voltdm,
>  		unsigned long volt);
>  void omap_voltage_register_pmic(struct omap_volt_pmic_info *pmic_info);
>  unsigned long omap_voltage_get_nom_volt(struct voltagedomain *voltdm);
> +int omap_voltage_add_userreq(struct voltagedomain *voltdm, struct device *dev,
> +		unsigned long *volt);
>  #ifdef CONFIG_PM
>  void omap_voltage_init_vc(struct omap_volt_vc_data *setup_vc);
>  void omap_change_voltscale_method(int voltscale_method);

Re: [PATCH 04/13] OMAP: Introduce API to return a device list associated with a voltage domain

[Kevin Hilman]

Thara Gopinath <thara@ti.com> writes:

> This patch adds an API in the opp layer that
> can be used by the voltage layer to get a list of all the
> scalable devices belonging to a particular voltage domain.
> This API is to be typically called only once by the voltage
> layer per voltage domain instance and the device list should
> be stored. This approach makes it easy during dvfs to scale
> all the devices associated with a voltage domain and then
> scale the voltage domain.
>
> Signed-off-by: Thara Gopinath <thara@ti.com>

I think this should be done in two steps.

During init, each OPP

> ---
>  arch/arm/plat-omap/include/plat/opp.h |    9 +++++++++
>  arch/arm/plat-omap/opp.c              |   28 ++++++++++++++++++++++++++++
>  2 files changed, 37 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/plat-omap/include/plat/opp.h b/arch/arm/plat-omap/include/plat/opp.h
> index 0e580ed..a4c1669 100644
> --- a/arch/arm/plat-omap/include/plat/opp.h
> +++ b/arch/arm/plat-omap/include/plat/opp.h
> @@ -18,6 +18,7 @@
>  #include <linux/cpufreq.h>
>  
>  #include <plat/common.h>
> +#include <plat/voltage.h>
>  
>  /**
>   * struct omap_opp_def - OMAP OPP Definition
> @@ -86,6 +87,9 @@ int opp_disable(struct omap_opp *opp);
>  
>  void opp_init_cpufreq_table(struct device *dev,
>  			    struct cpufreq_frequency_table **table);
> +
> +struct device **opp_init_voltage_params(struct voltagedomain *voltdm,
> +					int *dev_count);
>  #else
>  static inline unsigned long opp_get_voltage(const struct omap_opp *opp)
>  {
> @@ -149,5 +153,10 @@ void opp_init_cpufreq_table(struct omap_opp *opps,
>  {
>  }
>  
> +static inline struct device **opp_init_voltage_params(
> +			struct voltagedomain *voltdm, int *dev_count)
> +{
> +}
> +
>  #endif		/* CONFIG_PM */
>  #endif		/* __ASM_ARM_OMAP_OPP_H */
> diff --git a/arch/arm/plat-omap/opp.c b/arch/arm/plat-omap/opp.c
> index a3dea82..72dd62a 100644
> --- a/arch/arm/plat-omap/opp.c
> +++ b/arch/arm/plat-omap/opp.c
> @@ -502,3 +502,31 @@ void opp_init_cpufreq_table(struct device *dev,
>  
>  	*table = &freq_table[0];
>  }
> +
> +struct device **opp_init_voltage_params(struct voltagedomain *voltdm,
> +					int *dev_count)
> +{
> +	struct device_opp *dev_opp;
> +	struct device **dev_list;
> +	int count = 0, i = 0;
> +
> +	list_for_each_entry(dev_opp, &dev_opp_list, node) {
> +		if (!dev_opp->oh->vdd_name)
> +			continue;
> +
> +		if (!strcmp(dev_opp->oh->vdd_name, voltdm->name)) {
> +			dev_opp->oh->voltdm = voltdm;

Couldn't we assign the voltdm at opp_add() time since you added it as
part of the hwmod?

> +			count++;
> +		}
> +	}

So the above could be done at opp_add() time, leaving only the rest
for this function.

> +	dev_list = kzalloc(sizeof(struct device *) * count, GFP_KERNEL);
> +
> +	list_for_each_entry(dev_opp, &dev_opp_list, node) {
> +		if (dev_opp->oh->voltdm == voltdm)
> +			dev_list[i++] = dev_opp->dev;
> +	}
> +
> +	*dev_count = count;
> +	return dev_list;
> +}

Kevin

Re: [PATCH 04/13] OMAP: Introduce API to return a device list associated with a voltage domain

[Kevin Hilman]

On Fri, 2010-08-27 at 17:52 -0700, Kevin Hilman wrote:
> Thara Gopinath <thara@ti.com> writes:
> 
> > This patch adds an API in the opp layer that
> > can be used by the voltage layer to get a list of all the
> > scalable devices belonging to a particular voltage domain.
> > This API is to be typically called only once by the voltage
> > layer per voltage domain instance and the device list should
> > be stored. This approach makes it easy during dvfs to scale
> > all the devices associated with a voltage domain and then
> > scale the voltage domain.
> >
> > Signed-off-by: Thara Gopinath <thara@ti.com>
> 
> I think this should be done in two steps.
> 
> During init, each OPP

oops, ignore this part.  pushed send too soon.  Only comments below are
relevant.

Kevin

Re: [PATCH 01/13] OMAP: Introduce a user list for each voltage domain instance in the voltage driver.

[Kevin Hilman]

Kevin Hilman <khilman@deeprootsystems.com> writes:

>> +	if (!found) {
>> +		user = kzalloc(sizeof(struct omap_vdd_user_list), GFP_KERNEL);
>> +		if (!user) {
>> +			pr_err("%s: Unable to creat a new user for vdd_%s\n",
>> +				__func__, voltdm->name);
>> +			mutex_unlock(&vdd->scaling_mutex);
>> +			return -ENOMEM;
>> +		}
>> +		user->dev = dev;
>> +	} else {
>> +		plist_del(&user->node, &vdd->user_list);
>
> hmm... if we get here, we didn't find a match, so 'user' points to the
> last element in the list (which is the highest voltage request, I
> guess), or even NULL if the list is empty.  Then, this node is deleted.
>
> -ECONFUSED

OK, I'm a bit dense.

It took me a few reads, now I think I know what's going on here.

If we get here, we found a match, and you delete the existing value for
that device so that inserting a new one (with a new voltage) will be
kept in the right order.  Right?  

Some comments might help clarify things here.

Kevin

Re: [PATCH 10/13] OMAP3: Introduce custom set rate and get rate APIs for scalable devices

[Kevin Hilman]

Thara Gopinath <thara@ti.com> writes:

> This patch also introduces omap3_mpu_set_rate, omap3_iva_set_rate,
> omap3_l3_set_rate, omap3_mpu_get_rate, omap3_iva_get_rate,
> omap3_l3_get_rate as device specific set rate and get rate
> APIs for OMAP3 mpu, iva and l3_main devices. This patch also
> calls into opp_populate_rate_fns during system init to register
> various set_rate and get_rate APIs with the generic opp
> framework.
>
> Signed-off-by: Thara Gopinath <thara@ti.com>
> ---
>  arch/arm/mach-omap2/cpufreq34xx.c |  104 +++++++++++++++++++++++++++++++++++++
>  1 files changed, 104 insertions(+), 0 deletions(-)

These don't belong in cpufreq34xx.c (now known as opp3xxx_data.c)  This
file is only for the OPP data itself.  Any SoC specific code should go
into pm34xx.c.

Kevin

> diff --git a/arch/arm/mach-omap2/cpufreq34xx.c b/arch/arm/mach-omap2/cpufreq34xx.c
> index 195b5bc..8b9ef72 100644
> --- a/arch/arm/mach-omap2/cpufreq34xx.c
> +++ b/arch/arm/mach-omap2/cpufreq34xx.c
> @@ -24,8 +24,22 @@
>  
>  #include <plat/opp.h>
>  #include <plat/cpu.h>
> +#include <plat/clock.h>
> +
> +#include "cm-regbits-34xx.h"
> +#include "prm.h"
>  #include "omap3-opp.h"
>  
> +static int omap3_mpu_set_rate(struct device *dev, unsigned long rate);
> +static int omap3_iva_set_rate(struct device *dev, unsigned long rate);
> +static int omap3_l3_set_rate(struct device *dev, unsigned long rate);
> +
> +static unsigned long omap3_mpu_get_rate(struct device *dev);
> +static unsigned long omap3_iva_get_rate(struct device *dev);
> +static unsigned long omap3_l3_get_rate(struct device *dev);
> +
> +struct clk *dpll1_clk, *dpll2_clk, *dpll3_clk;
> +
>  static struct omap_opp_def __initdata omap34xx_opp_def_list[] = {
>  	/* MPU OPP1 */
>  	OMAP_OPP_DEF("mpu", true, 125000000, 975000),
> @@ -92,12 +106,82 @@ static struct omap_opp_def __initdata omap36xx_opp_def_list[] = {
>  };
>  static u32 omap36xx_opp_def_size = ARRAY_SIZE(omap36xx_opp_def_list);
>  
> +
> +static int omap3_mpu_set_rate(struct device *dev, unsigned long rate)
> +{
> +	unsigned long cur_rate = omap3_mpu_get_rate(dev);
> +	int ret;
> +
> +#ifdef CONFIG_CPU_FREQ
> +	struct cpufreq_freqs freqs_notify;
> +
> +	freqs_notify.old = cur_rate / 1000;
> +	freqs_notify.new = rate / 1000;
> +	freqs_notify.cpu = 0;
> +	/* Send pre notification to CPUFreq */
> +	cpufreq_notify_transition(&freqs_notify, CPUFREQ_PRECHANGE);
> +#endif
> +	ret = clk_set_rate(dpll1_clk, rate);
> +	if (ret) {
> +		dev_warn(dev, "%s: Unable to set rate to %ld\n",
> +			__func__, rate);
> +		return ret;
> +	}
> +
> +#ifdef CONFIG_CPU_FREQ
> +	/* Send a post notification to CPUFreq */
> +	cpufreq_notify_transition(&freqs_notify, CPUFREQ_POSTCHANGE);
> +#endif
> +
> +#ifndef CONFIG_CPU_FREQ
> +	/*Update loops_per_jiffy if processor speed is being changed*/
> +	loops_per_jiffy = compute_lpj(loops_per_jiffy,
> +			cur_rate / 1000, rate / 1000);
> +#endif
> +	return 0;
> +}
> +
> +static unsigned long omap3_mpu_get_rate(struct device *dev)
> +{
> +	return dpll1_clk->rate;
> +}
> +
> +static int omap3_iva_set_rate(struct device *dev, unsigned long rate)
> +{
> +	return clk_set_rate(dpll2_clk, rate);
> +}
> +
> +static unsigned long omap3_iva_get_rate(struct device *dev)
> +{
> +	return dpll2_clk->rate;
> +}
> +
> +static int omap3_l3_set_rate(struct device *dev, unsigned long rate)
> +{
> +	int l3_div;
> +
> +	l3_div = cm_read_mod_reg(CORE_MOD, CM_CLKSEL) &
> +			OMAP3430_CLKSEL_L3_MASK;
> +
> +	return clk_set_rate(dpll3_clk, rate * l3_div);
> +}
> +
> +static unsigned long omap3_l3_get_rate(struct device *dev)
> +{
> +	int l3_div;
> +
> +	l3_div = cm_read_mod_reg(CORE_MOD, CM_CLKSEL) &
> +			OMAP3430_CLKSEL_L3_MASK;
> +	return dpll3_clk->rate / l3_div;
> +}
> +
>  /* Temp variable to allow multiple calls */
>  static u8 __initdata omap3_table_init;
>  
>  int __init omap3_pm_init_opp_table(void)
>  {
>  	struct omap_opp_def *opp_def, *omap3_opp_def_list;
> +	struct device *dev;
>  	u32 omap3_opp_def_size;
>  	int i, r;
>  
> @@ -122,6 +206,26 @@ int __init omap3_pm_init_opp_table(void)
>  			       opp_def->freq, opp_def->hwmod_name);
>  	}
>  
> +	dpll1_clk = clk_get(NULL, "dpll1_ck");
> +	dpll2_clk = clk_get(NULL, "dpll2_ck");
> +	dpll3_clk = clk_get(NULL, "dpll3_m2_ck");
> +
> +	/* Populate the set rate and get rate for mpu, iva and l3 device */
> +	dev = omap2_get_mpuss_device();
> +	if (dev)
> +		opp_populate_rate_fns(dev, omap3_mpu_set_rate,
> +				omap3_mpu_get_rate);
> +
> +	dev = omap2_get_iva_device();
> +	if (dev)
> +		opp_populate_rate_fns(dev, omap3_iva_set_rate,
> +				omap3_iva_get_rate);
> +
> +	dev = omap2_get_l3_device();
> +	if (dev)
> +		opp_populate_rate_fns(dev, omap3_l3_set_rate,
> +				omap3_l3_get_rate);
> +
>  	return 0;
>  }

Re: [PATCH 01/13] OMAP: Introduce a user list for each voltage domain instance in the voltage driver.

[Kevin Hilman]

Thara Gopinath <thara@ti.com> writes:

> This patch introduces a user list of devices associated with each
> voltage domain instance. The user list is implemented using plist
> structure with priority node populated with the voltage values.
> This patch also adds an API which will take in a device and
> requested voltage as parameters, adds the info to the user list
> and returns back the maximum voltage requested by all the user
> devices. This can be used anytime to get the voltage that the
> voltage domain instance can be transitioned into.
>
> Signed-off-by: Thara Gopinath <thara@ti.com>

Looking closer at this, keeping track of a list of devices and
constraints is what the regulator framework does as well.  

Before we get too far down this path, we need to start working with
Thomas Petazzoni to better understand how we can use the regulator
framework for much of the management levels of the voltage layer.

I'd rather not re-invent some of the management/constraints management
that could be done by the regulator framework.

Basically, I think

  r = regulator_get(dev, voltdm->name)
  regulator_set_voltage(r, volt)

would basially be the equivalent of

  omap_voltage_add_userreq(voldm, dev, &volt);
  omap_scale_voltage(voltdm, volt)

The regulator framework not only provides management of the users and
constraints, but also provides post-change notifiers where things like
dependent voltages could be handled.

Kevin

Re: [PATCH 04/13] OMAP: Introduce API to return a device list associated with a voltage domain

[Kevin Hilman]

Thara Gopinath <thara@ti.com> writes:

> This patch adds an API in the opp layer that
> can be used by the voltage layer to get a list of all the
> scalable devices belonging to a particular voltage domain.
> This API is to be typically called only once by the voltage
> layer per voltage domain instance and the device list should
> be stored. This approach makes it easy during dvfs to scale
> all the devices associated with a voltage domain and then
> scale the voltage domain.
>
> Signed-off-by: Thara Gopinath <thara@ti.com>

I don't think the OPP layer is the right place for this after all.

How about something like this in the voltage layer:

  omap_voltage_add_device(struct voltagedomain *voltdm, struct device *dev)

During omap_device_build(), if the hwmod has a voltage domain, it
calls this function to register it with the voltage layer.

This function then creates a list (internal to voltage layer) of all the
devices in a voltage domain rather than having to query the OPP layer
for it.

Kevin

> ---
>  arch/arm/plat-omap/include/plat/opp.h |    9 +++++++++
>  arch/arm/plat-omap/opp.c              |   28 ++++++++++++++++++++++++++++
>  2 files changed, 37 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/plat-omap/include/plat/opp.h b/arch/arm/plat-omap/include/plat/opp.h
> index 0e580ed..a4c1669 100644
> --- a/arch/arm/plat-omap/include/plat/opp.h
> +++ b/arch/arm/plat-omap/include/plat/opp.h
> @@ -18,6 +18,7 @@
>  #include <linux/cpufreq.h>
>  
>  #include <plat/common.h>
> +#include <plat/voltage.h>
>  
>  /**
>   * struct omap_opp_def - OMAP OPP Definition
> @@ -86,6 +87,9 @@ int opp_disable(struct omap_opp *opp);
>  
>  void opp_init_cpufreq_table(struct device *dev,
>  			    struct cpufreq_frequency_table **table);
> +
> +struct device **opp_init_voltage_params(struct voltagedomain *voltdm,
> +					int *dev_count);
>  #else
>  static inline unsigned long opp_get_voltage(const struct omap_opp *opp)
>  {
> @@ -149,5 +153,10 @@ void opp_init_cpufreq_table(struct omap_opp *opps,
>  {
>  }
>  
> +static inline struct device **opp_init_voltage_params(
> +			struct voltagedomain *voltdm, int *dev_count)
> +{
> +}
> +
>  #endif		/* CONFIG_PM */
>  #endif		/* __ASM_ARM_OMAP_OPP_H */
> diff --git a/arch/arm/plat-omap/opp.c b/arch/arm/plat-omap/opp.c
> index a3dea82..72dd62a 100644
> --- a/arch/arm/plat-omap/opp.c
> +++ b/arch/arm/plat-omap/opp.c
> @@ -502,3 +502,31 @@ void opp_init_cpufreq_table(struct device *dev,
>  
>  	*table = &freq_table[0];
>  }
> +
> +struct device **opp_init_voltage_params(struct voltagedomain *voltdm,
> +					int *dev_count)
> +{
> +	struct device_opp *dev_opp;
> +	struct device **dev_list;
> +	int count = 0, i = 0;
> +
> +	list_for_each_entry(dev_opp, &dev_opp_list, node) {
> +		if (!dev_opp->oh->vdd_name)
> +			continue;
> +
> +		if (!strcmp(dev_opp->oh->vdd_name, voltdm->name)) {
> +			dev_opp->oh->voltdm = voltdm;
> +			count++;
> +		}
> +	}
> +
> +	dev_list = kzalloc(sizeof(struct device *) * count, GFP_KERNEL);
> +
> +	list_for_each_entry(dev_opp, &dev_opp_list, node) {
> +		if (dev_opp->oh->voltdm == voltdm)
> +			dev_list[i++] = dev_opp->dev;
> +	}
> +
> +	*dev_count = count;
> +	return dev_list;
> +}

Re: [PATCH 01/13] OMAP: Introduce a user list for each voltage domain instance in the voltage driver.

[Thomas Petazzoni]

Hello,

On Wed, 01 Sep 2010 15:51:40 -0700
Kevin Hilman <khilman@deeprootsystems.com> wrote:

> Looking closer at this, keeping track of a list of devices and
> constraints is what the regulator framework does as well.  
> 
> Before we get too far down this path, we need to start working with
> Thomas Petazzoni to better understand how we can use the regulator
> framework for much of the management levels of the voltage layer.

Yes, as discussed on IRC with Kevin, I think that some of this voltage
layer mechanisms would benefit from using the existing kernel regulator
framework.

The regulator framework already keeps tracks of consumers (in your
patch set called "vdd users"), and for each consumer, keeps track of
the requested voltage. The maximum requested voltage is then applied to
the regulator. It seems to fit quite well some of the mechanisms you're
introducing in this patch set.

Regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [PATCH 01/13] OMAP: Introduce a user list for each voltage domain instance in the voltage driver.

[Nishanth Menon]

Thomas Petazzoni had written, on 09/02/2010 02:43 AM, the following:
> Hello,
> 
> On Wed, 01 Sep 2010 15:51:40 -0700
> Kevin Hilman <khilman@deeprootsystems.com> wrote:
> 
>> Looking closer at this, keeping track of a list of devices and
>> constraints is what the regulator framework does as well.  
>>
>> Before we get too far down this path, we need to start working with
>> Thomas Petazzoni to better understand how we can use the regulator
>> framework for much of the management levels of the voltage layer.
> 
> Yes, as discussed on IRC with Kevin, I think that some of this voltage
> layer mechanisms would benefit from using the existing kernel regulator
> framework.
> 
> The regulator framework already keeps tracks of consumers (in your
> patch set called "vdd users"), and for each consumer, keeps track of
> the requested voltage. The maximum requested voltage is then applied to
> the regulator. It seems to fit quite well some of the mechanisms you're
> introducing in this patch set.

Just brainstorming -> if we use the regulator framework - there are 
potential benefits - agreed. BUT, consider the cpuidle path -> currently 
we disable SR while hitting off/ret for class3, this is done in irq 
locked context while the regulator framework uses locks by itself - we 
would probably have to evolve an entirely different mechanism to handle 
this.

SR by itself can easily be represented I believe and my thoughts  when i 
initialy looked at that option had been:
a) latency overheads
b) flexibility we achieve vs complexity in s/w implementation
c) lock handling - esp impact on omap_sram_idle paths..

-- 
Regards,
Nishanth Menon

PS:personally though concept of latency additions when scaling voltages, 
disabling SR etc should be a parameter in userspace governor decisions 
(the fact that cpuidle and cpufreq are independent statemachine is not 
my personal fav either). But this is a larger topic of discussion not 
pertinent to this thread..

Re: [PATCH 01/13] OMAP: Introduce a user list for each voltage domain instance in the voltage driver.

[Felipe Balbi]

On Thu, 2 Sep 2010 03:17:56 -0500, Nishanth Menon <nm@ti.com> wrote:
> Just brainstorming -> if we use the regulator framework - there are 
> potential benefits - agreed. BUT, consider the cpuidle path -> currently

> we disable SR while hitting off/ret for class3, this is done in irq 
> locked context while the regulator framework uses locks by itself - we 

wouldn't it be enough to:

spin_unlock(&sr_lock);
call_whatever_regulator_api();
spin_lock(&sr_lock);

[...]

spin_unlock_irqrestore(&sr_lock, flags);

??

otherwise we will have yet another OMAP-only API to maintain.

-- 
balbi

Re: [PATCH 01/13] OMAP: Introduce a user list for each voltage domain instance in the voltage driver.

[Nishanth Menon]

Felipe Balbi had written, on 09/02/2010 05:00 AM, the following:
> On Thu, 2 Sep 2010 03:17:56 -0500, Nishanth Menon <nm@ti.com> wrote:
>> Just brainstorming -> if we use the regulator framework - there are 
>> potential benefits - agreed. BUT, consider the cpuidle path -> currently
> 
>> we disable SR while hitting off/ret for class3, this is done in irq 
>> locked context while the regulator framework uses locks by itself - we 
> 
> wouldn't it be enough to:
> 
> spin_unlock(&sr_lock);
> call_whatever_regulator_api();
> spin_lock(&sr_lock);
> 
> [...]
> 
> spin_unlock_irqrestore(&sr_lock, flags);
> 
> ??
> 
> otherwise we will have yet another OMAP-only API to maintain.
> 
unfortunately no. look at omap_sram_idle function in 
arch/arm/mach-omap2/pm34xx.c

we do irq_lock_save to prevent interrupts from messing up our decision logic

apply a lot of checks to translate C state to mean which domain is going 
to go to which OMAP domain state mode - ret/off/inactive etc..

add a bunch of erratas on top of it

finally see if the core/mpu domains (in case of omap4 ivahd domain as 
well) is going to ret/off state - if yes, SR is disabled for that domain.

note - if we allow unlock of irqs at this point, we cannot predictably 
progress down the logic.

the option is to move up the sr disable out of omap_sram_idle into yet 
to be determined logic where irqs are enabled, c state is decided and if 
c state is lower than a threshold (meaning if mpu OR core OR ivahd can 
go to lower power states), disable SR.. not efficient and without 
looking deeper at logic and considering multiple omap generations not 
sure if this will scale either..


-- 
Regards,
Nishanth Menon

Re: [PATCH 01/13] OMAP: Introduce a user list for each voltage domain instance in the voltage driver.

[Felipe Balbi]

Hi,

On Thu, 2 Sep 2010 05:17:01 -0500, Nishanth Menon <nm@ti.com> wrote:
> note - if we allow unlock of irqs at this point, we cannot predictably 
> progress down the logic.

spin_unlock() would not re-enable IRQs, would it ? Isn't it so that
spin_unlock_irq() would be the one re-enabling IRQ ?

-- 
balbi

Re: [PATCH 01/13] OMAP: Introduce a user list for each voltage domain instance in the voltage driver.

[Nishanth Menon]

Felipe Balbi had written, on 09/02/2010 05:28 AM, the following:
> Hi,
> 
> On Thu, 2 Sep 2010 05:17:01 -0500, Nishanth Menon <nm@ti.com> wrote:
>> note - if we allow unlock of irqs at this point, we cannot predictably 
>> progress down the logic.
> 
> spin_unlock() would not re-enable IRQs, would it ? Isn't it so that
> spin_unlock_irq() would be the one re-enabling IRQ ?
> 
oopss.. my bad.. if we were to do regulator based implementation of 
voltage framework, looking closer at the code, driver/regulator/core.c 
-> rdev->mutex is held for set_voltage, set_mode and all entry functions 
for regulator operations -> this would be the only concern i have.. I 
may be barking up the wrong tree here, but i think if i read 
Documentation/mutex-design.txt right, "contexts such as tasklets and 
timers" and "mutexes may not be used in hardware or software interrupt" 
means to me dont do this in irq locked context such as the sitn in 
omap_sram_idle?

-- 
Regards,
Nishanth Menon

Re: [PATCH 01/13] OMAP: Introduce a user list for each voltage domain instance in the voltage driver.

[Felipe Balbi]

Hi,

On Thu, 2 Sep 2010 05:40:56 -0500, Nishanth Menon <nm@ti.com> wrote:
> Felipe Balbi had written, on 09/02/2010 05:28 AM, the following:
>> Hi,
>> 
>> On Thu, 2 Sep 2010 05:17:01 -0500, Nishanth Menon <nm@ti.com> wrote:
>>> note - if we allow unlock of irqs at this point, we cannot predictably

>>> progress down the logic.
>> 
>> spin_unlock() would not re-enable IRQs, would it ? Isn't it so that
>> spin_unlock_irq() would be the one re-enabling IRQ ?
>> 
> oopss.. my bad.. if we were to do regulator based implementation of 
> voltage framework, looking closer at the code, driver/regulator/core.c 
> -> rdev->mutex is held for set_voltage, set_mode and all entry functions

> for regulator operations -> this would be the only concern i have.. I 
> may be barking up the wrong tree here, but i think if i read 
> Documentation/mutex-design.txt right, "contexts such as tasklets and 
> timers" and "mutexes may not be used in hardware or software interrupt" 
> means to me dont do this in irq locked context such as the sitn in 
> omap_sram_idle?

true, some re-work would have to be done if you want to use requlator
framework.

-- 
balbi

Re: [PATCH 01/13] OMAP: Introduce a user list for each voltage domain instance in the voltage driver.

[Kevin Hilman]

Nishanth Menon <nm@ti.com> writes:

> Thomas Petazzoni had written, on 09/02/2010 02:43 AM, the following:
>> Hello,
>>
>> On Wed, 01 Sep 2010 15:51:40 -0700
>> Kevin Hilman <khilman@deeprootsystems.com> wrote:
>>
>>> Looking closer at this, keeping track of a list of devices and
>>> constraints is what the regulator framework does as well.  
>>>
>>> Before we get too far down this path, we need to start working with
>>> Thomas Petazzoni to better understand how we can use the regulator
>>> framework for much of the management levels of the voltage layer.
>>
>> Yes, as discussed on IRC with Kevin, I think that some of this voltage
>> layer mechanisms would benefit from using the existing kernel regulator
>> framework.
>>
>> The regulator framework already keeps tracks of consumers (in your
>> patch set called "vdd users"), and for each consumer, keeps track of
>> the requested voltage. The maximum requested voltage is then applied to
>> the regulator. It seems to fit quite well some of the mechanisms you're
>> introducing in this patch set.
>
> Just brainstorming -> if we use the regulator framework - there are
> potential benefits - agreed. BUT, consider the cpuidle path ->
> currently we disable SR while hitting off/ret for class3, this is done
> in irq locked context while the regulator framework uses locks by
> itself - we would probably have to evolve an entirely different
> mechanism to handle this.
>
> SR by itself can easily be represented I believe and my thoughts  when
> i initialy looked at that option had been:
> a) latency overheads
> b) flexibility we achieve vs complexity in s/w implementation
> c) lock handling - esp impact on omap_sram_idle paths..

If you look at the current PM branch (specifically pm-sr), you'll see
that with the updated SR layer, there is no SR enable/disable in the
idle path because there is no voltage scaling during idle.

That being said, even if this is add later, the idle path can potentialy
call the SR API directly if needed for the enable/disable.

The concern Thomas and I are raising is that we don't want to re-invent
regulator framework functionality in the OMAP voltage layer.  Rather, we
should keep the OMAP voltage layer as the part that touches the HW, but
use the regulator framework for the higher-level management of users and
constraints.

Performance issues can be addressed as we hit them, but at this level of
the design, I want to make sure the frameworks make sense.

Kevin

Re: [PATCH 01/13] OMAP: Introduce a user list for each voltage domain instance in the voltage driver.

[Nishanth Menon]

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

Kevin Hilman had written, on 09/02/2010 12:47 PM, the following:
> Nishanth Menon <nm@ti.com> writes:
> 
>> Thomas Petazzoni had written, on 09/02/2010 02:43 AM, the following:
>>> Hello,
>>>
>>> On Wed, 01 Sep 2010 15:51:40 -0700
>>> Kevin Hilman <khilman@deeprootsystems.com> wrote:
>>>
>>>> Looking closer at this, keeping track of a list of devices and
>>>> constraints is what the regulator framework does as well.  
>>>>
>>>> Before we get too far down this path, we need to start working with
>>>> Thomas Petazzoni to better understand how we can use the regulator
>>>> framework for much of the management levels of the voltage layer.
>>> Yes, as discussed on IRC with Kevin, I think that some of this voltage
>>> layer mechanisms would benefit from using the existing kernel regulator
>>> framework.
>>>
>>> The regulator framework already keeps tracks of consumers (in your
>>> patch set called "vdd users"), and for each consumer, keeps track of
>>> the requested voltage. The maximum requested voltage is then applied to
>>> the regulator. It seems to fit quite well some of the mechanisms you're
>>> introducing in this patch set.
>> Just brainstorming -> if we use the regulator framework - there are
>> potential benefits - agreed. BUT, consider the cpuidle path ->
>> currently we disable SR while hitting off/ret for class3, this is done
>> in irq locked context while the regulator framework uses locks by
>> itself - we would probably have to evolve an entirely different
>> mechanism to handle this.
>>
>> SR by itself can easily be represented I believe and my thoughts  when
>> i initialy looked at that option had been:
>> a) latency overheads
>> b) flexibility we achieve vs complexity in s/w implementation
>> c) lock handling - esp impact on omap_sram_idle paths..
> 
> If you look at the current PM branch (specifically pm-sr), you'll see
> that with the updated SR layer, there is no SR enable/disable in the
> idle path because there is no voltage scaling during idle.
This is interesting and I wonder if it works in reality, class 3 operation:
you enable SR h/w monitoring loop as part of dvfs, and when you hit 
cpuidle and you discover that domain x is going to idle,
you'd do:
omap_sram_idle() {
	figureout a lot of stuff
	if (core_next_state == OFF)
		disable_sr(core);
	if (mpu_next_state == OFF)
		disable_sr(mpu);
	__omap_sram_idle()
	if (core_next_state == OFF)
		enable_sr(core);
	if (mpu_next_state == OFF)
		enable_sr(mpu);

}

disable_sr(domain){
disable hw loop
forceupdate to nominal voltage
}
enable_sr(domain){
enable hw loop
}

in the days of SRF, we used to have a patch as well.. see one of the 
first versions:
http://marc.info/?l=linux-omap&m=122000411826768&w=2


> 
> That being said, even if this is add later, the idle path can potentialy
> call the SR API directly if needed for the enable/disable.
not clean if it was directly implemented by regulator framework

> 
> The concern Thomas and I are raising is that we don't want to re-invent
> regulator framework functionality in the OMAP voltage layer.  Rather, we
> should keep the OMAP voltage layer as the part that touches the HW, but
> use the regulator framework for the higher-level management of users and
> constraints.
I completely like the idea of going down the regulator path for voltage 
management, only brainstorming on how to make that happen.


btw, I was playing around with something(attached) that "might" show how 
it might be done by enhancing the regulator framework for folks who may 
want to manage their own exclusivity.. e.g. see attachment..

note: I am not saying this is the way to do it, just that: we need to 
allow regulator operations in irq_locked context if we want to do SR 
operations (including voltage transitions)

> 
> Performance issues can be addressed as we hit them, but at this level of
> the design, I want to make sure the frameworks make sense.
I agree.

> 
> Kevin
> 


-- 
Regards,
Nishanth Menon

[-- Attachment #2: 0001-regulator-core-change-exclusive-to-flags.patch --]
[-- Type: text/x-patch, Size: 2044 bytes --]

>From 296cd903f0ce5b70e23a052515d8de1ebb9a15cd Mon Sep 17 00:00:00 2001
From: Nishanth Menon <nm@ti.com>
Date: Thu, 2 Sep 2010 13:24:35 -0500
Subject: {HACK}[PATCH 1/2] regulator: core: change exclusive to flags

we currently use an int for exclusivity, instead change it to
a flag based usage. this allows us to extend this to additional flags
as well

Signed-off-by: Nishanth Menon <nm@ti.com>
---
this patch is meant to be for demonstration purposes only.

 drivers/regulator/core.c         |    6 +++---
 include/linux/regulator/driver.h |    4 +++-
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 422a709..0a1e199 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1127,7 +1127,7 @@ static struct regulator *_regulator_get(struct device *dev, const char *id,
 	return regulator;
 
 found:
-	if (rdev->exclusive) {
+	if (rdev->flags & REGULATOR_EXCLUSIVE_FLAG) {
 		regulator = ERR_PTR(-EPERM);
 		goto out;
 	}
@@ -1148,7 +1148,7 @@ found:
 
 	rdev->open_count++;
 	if (exclusive) {
-		rdev->exclusive = 1;
+		rdev->flags |= REGULATOR_EXCLUSIVE_FLAG;
 
 		ret = _regulator_is_enabled(rdev);
 		if (ret > 0)
@@ -1238,7 +1238,7 @@ void regulator_put(struct regulator *regulator)
 	kfree(regulator);
 
 	rdev->open_count--;
-	rdev->exclusive = 0;
+	rdev->flags &= ~REGULATOR_EXCLUSIVE_FLAG;
 
 	module_put(rdev->owner);
 	mutex_unlock(&regulator_list_mutex);
diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index 592cd7c..6397ab3 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -156,6 +156,8 @@ struct regulator_desc {
 	struct module *owner;
 };
 
+#define REGULATOR_EXCLUSIVE_FLAG	(0x1 << 1)
+
 /*
  * struct regulator_dev
  *
@@ -170,7 +172,7 @@ struct regulator_dev {
 	struct regulator_desc *desc;
 	int use_count;
 	int open_count;
-	int exclusive;
+	unsigned int flags;
 
 	/* lists we belong to */
 	struct list_head list; /* list of all regulators */
-- 
1.6.3.3


[-- Attachment #3: 0002-regulator-core-introduce-unsafe-operations.patch --]
[-- Type: text/x-patch, Size: 12349 bytes --]

>From 3145fa45029fbdbfed2b2a2a6e335a72f8c47d36 Mon Sep 17 00:00:00 2001
From: Nishanth Menon <nm@ti.com>
Date: Thu, 2 Sep 2010 13:40:40 -0500
Subject: {HACK}[PATCH 2/2] regulator: core: introduce unsafe operations

introduce unsafe operations for consumers who can manage
their own operation sequencing and dont want core to bother
locking out multiple accesses exclusive of each other.

Signed-off-by: Nishanth Menon <nm@ti.com>
---
this patch is for demonstration purpose only

 drivers/regulator/core.c           |  121 ++++++++++++++++++++++++++----------
 include/linux/regulator/consumer.h |    2 +
 include/linux/regulator/driver.h   |    1 +
 3 files changed, 91 insertions(+), 33 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 0a1e199..c4c0e98 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -223,9 +223,11 @@ static ssize_t regulator_uV_show(struct device *dev,
 	struct regulator_dev *rdev = dev_get_drvdata(dev);
 	ssize_t ret;
 
-	mutex_lock(&rdev->mutex);
+	if (!(rdev->flags & REGULATOR_UNSAFEOPS_FLAG))
+		mutex_lock(&rdev->mutex);
 	ret = sprintf(buf, "%d\n", _regulator_get_voltage(rdev));
-	mutex_unlock(&rdev->mutex);
+	if (!(rdev->flags & REGULATOR_UNSAFEOPS_FLAG))
+		mutex_unlock(&rdev->mutex);
 
 	return ret;
 }
@@ -288,9 +290,11 @@ static ssize_t regulator_state_show(struct device *dev,
 	struct regulator_dev *rdev = dev_get_drvdata(dev);
 	ssize_t ret;
 
-	mutex_lock(&rdev->mutex);
+	if (!(rdev->flags & REGULATOR_UNSAFEOPS_FLAG))
+		mutex_lock(&rdev->mutex);
 	ret = regulator_print_state(buf, _regulator_is_enabled(rdev));
-	mutex_unlock(&rdev->mutex);
+	if (!(rdev->flags & REGULATOR_UNSAFEOPS_FLAG))
+		mutex_unlock(&rdev->mutex);
 
 	return ret;
 }
@@ -392,10 +396,12 @@ static ssize_t regulator_total_uA_show(struct device *dev,
 	struct regulator *regulator;
 	int uA = 0;
 
-	mutex_lock(&rdev->mutex);
+	if (!(rdev->flags & REGULATOR_UNSAFEOPS_FLAG))
+		mutex_lock(&rdev->mutex);
 	list_for_each_entry(regulator, &rdev->consumer_list, list)
 		uA += regulator->uA_load;
-	mutex_unlock(&rdev->mutex);
+	if (!(rdev->flags & REGULATOR_UNSAFEOPS_FLAG))
+		mutex_unlock(&rdev->mutex);
 	return sprintf(buf, "%d\n", uA);
 }
 static DEVICE_ATTR(requested_microamps, 0444, regulator_total_uA_show, NULL);
@@ -1078,7 +1084,7 @@ static int _regulator_get_enable_time(struct regulator_dev *rdev)
 
 /* Internal regulator request function */
 static struct regulator *_regulator_get(struct device *dev, const char *id,
-					int exclusive)
+					int exclusive, int unsafe)
 {
 	struct regulator_dev *rdev;
 	struct regulator_map *map;
@@ -1156,6 +1162,8 @@ found:
 		else
 			rdev->use_count = 0;
 	}
+	if (unsafe)
+		rdev->flags |= REGULATOR_UNSAFEOPS_FLAG;
 
 out:
 	mutex_unlock(&regulator_list_mutex);
@@ -1178,7 +1186,7 @@ out:
  */
 struct regulator *regulator_get(struct device *dev, const char *id)
 {
-	return _regulator_get(dev, id, 0);
+	return _regulator_get(dev, id, 0, 0);
 }
 EXPORT_SYMBOL_GPL(regulator_get);
 
@@ -1205,11 +1213,27 @@ EXPORT_SYMBOL_GPL(regulator_get);
  */
 struct regulator *regulator_get_exclusive(struct device *dev, const char *id)
 {
-	return _regulator_get(dev, id, 1);
+	return _regulator_get(dev, id, 1, 0);
 }
 EXPORT_SYMBOL_GPL(regulator_get_exclusive);
 
 /**
+ * regulator_get_exclusive_unsafe() - completely unsafe operations!
+ * @dev: device for regulator "consumer"
+ * @id: Supply name or regulator ID.
+ *
+ * Use of this function is strictly dangerous - caller should maintain
+ * exclusivity of access between operations, core will not take care of
+ * the same. this operation will fail unless the regulator can be
+ * exclusively openend
+ */
+struct regulator *regulator_get_exclusive_unsafe(struct device *dev,
+		const char *id)
+{
+	return _regulator_get(dev, id, 1, 1);
+}
+EXPORT_SYMBOL_GPL(regulator_get_exclusive);
+/**
  * regulator_put - "free" the regulator source
  * @regulator: regulator source
  *
@@ -1239,6 +1263,7 @@ void regulator_put(struct regulator *regulator)
 
 	rdev->open_count--;
 	rdev->flags &= ~REGULATOR_EXCLUSIVE_FLAG;
+	rdev->flags &= ~REGULATOR_UNSAFEOPS_FLAG;
 
 	module_put(rdev->owner);
 	mutex_unlock(&regulator_list_mutex);
@@ -1340,9 +1365,11 @@ int regulator_enable(struct regulator *regulator)
 	struct regulator_dev *rdev = regulator->rdev;
 	int ret = 0;
 
-	mutex_lock(&rdev->mutex);
+	if (!(rdev->flags & REGULATOR_UNSAFEOPS_FLAG))
+		mutex_lock(&rdev->mutex);
 	ret = _regulator_enable(rdev);
-	mutex_unlock(&rdev->mutex);
+	if (!(rdev->flags & REGULATOR_UNSAFEOPS_FLAG))
+		mutex_unlock(&rdev->mutex);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(regulator_enable);
@@ -1409,9 +1436,11 @@ int regulator_disable(struct regulator *regulator)
 	struct regulator_dev *rdev = regulator->rdev;
 	int ret = 0;
 
-	mutex_lock(&rdev->mutex);
+	if (!(rdev->flags & REGULATOR_UNSAFEOPS_FLAG))
+		mutex_lock(&rdev->mutex);
 	ret = _regulator_disable(rdev);
-	mutex_unlock(&rdev->mutex);
+	if (!(rdev->flags & REGULATOR_UNSAFEOPS_FLAG))
+		mutex_unlock(&rdev->mutex);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(regulator_disable);
@@ -1456,10 +1485,12 @@ int regulator_force_disable(struct regulator *regulator)
 {
 	int ret;
 
-	mutex_lock(&regulator->rdev->mutex);
+	if (!(rdev->flags & REGULATOR_UNSAFEOPS_FLAG))
+		mutex_lock(&regulator->rdev->mutex);
 	regulator->uA_load = 0;
 	ret = _regulator_force_disable(regulator->rdev);
-	mutex_unlock(&regulator->rdev->mutex);
+	if (!(rdev->flags & REGULATOR_UNSAFEOPS_FLAG))
+		mutex_unlock(&regulator->rdev->mutex);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(regulator_force_disable);
@@ -1489,9 +1520,11 @@ int regulator_is_enabled(struct regulator *regulator)
 {
 	int ret;
 
-	mutex_lock(&regulator->rdev->mutex);
+	if (!(rdev->flags & REGULATOR_UNSAFEOPS_FLAG))
+		mutex_lock(&regulator->rdev->mutex);
 	ret = _regulator_is_enabled(regulator->rdev);
-	mutex_unlock(&regulator->rdev->mutex);
+	if (!(rdev->flags & REGULATOR_UNSAFEOPS_FLAG))
+		mutex_unlock(&regulator->rdev->mutex);
 
 	return ret;
 }
@@ -1532,9 +1565,11 @@ int regulator_list_voltage(struct regulator *regulator, unsigned selector)
 	if (!ops->list_voltage || selector >= rdev->desc->n_voltages)
 		return -EINVAL;
 
-	mutex_lock(&rdev->mutex);
+	if (!(rdev->flags & REGULATOR_UNSAFEOPS_FLAG))
+		mutex_lock(&rdev->mutex);
 	ret = ops->list_voltage(rdev, selector);
-	mutex_unlock(&rdev->mutex);
+	if (!(rdev->flags & REGULATOR_UNSAFEOPS_FLAG))
+		mutex_unlock(&rdev->mutex);
 
 	if (ret > 0) {
 		if (ret < rdev->constraints->min_uV)
@@ -1599,7 +1634,8 @@ int regulator_set_voltage(struct regulator *regulator, int min_uV, int max_uV)
 	struct regulator_dev *rdev = regulator->rdev;
 	int ret;
 
-	mutex_lock(&rdev->mutex);
+	if (!(rdev->flags & REGULATOR_UNSAFEOPS_FLAG))
+		mutex_lock(&rdev->mutex);
 
 	/* sanity check */
 	if (!rdev->desc->ops->set_voltage) {
@@ -1617,7 +1653,8 @@ int regulator_set_voltage(struct regulator *regulator, int min_uV, int max_uV)
 
 out:
 	_notifier_call_chain(rdev, REGULATOR_EVENT_VOLTAGE_CHANGE, NULL);
-	mutex_unlock(&rdev->mutex);
+	if (!(rdev->flags & REGULATOR_UNSAFEOPS_FLAG))
+		mutex_unlock(&rdev->mutex);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(regulator_set_voltage);
@@ -1644,11 +1681,13 @@ int regulator_get_voltage(struct regulator *regulator)
 {
 	int ret;
 
-	mutex_lock(&regulator->rdev->mutex);
+	if (!(rdev->flags & REGULATOR_UNSAFEOPS_FLAG))
+		mutex_lock(&regulator->rdev->mutex);
 
 	ret = _regulator_get_voltage(regulator->rdev);
 
-	mutex_unlock(&regulator->rdev->mutex);
+	if (!(rdev->flags & REGULATOR_UNSAFEOPS_FLAG))
+		mutex_unlock(&regulator->rdev->mutex);
 
 	return ret;
 }
@@ -1676,7 +1715,8 @@ int regulator_set_current_limit(struct regulator *regulator,
 	struct regulator_dev *rdev = regulator->rdev;
 	int ret;
 
-	mutex_lock(&rdev->mutex);
+	if (!(rdev->flags & REGULATOR_UNSAFEOPS_FLAG))
+		mutex_lock(&rdev->mutex);
 
 	/* sanity check */
 	if (!rdev->desc->ops->set_current_limit) {
@@ -1691,7 +1731,8 @@ int regulator_set_current_limit(struct regulator *regulator,
 
 	ret = rdev->desc->ops->set_current_limit(rdev, min_uA, max_uA);
 out:
-	mutex_unlock(&rdev->mutex);
+	if (!(rdev->flags & REGULATOR_UNSAFEOPS_FLAG))
+		mutex_unlock(&rdev->mutex);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(regulator_set_current_limit);
@@ -1700,7 +1741,8 @@ static int _regulator_get_current_limit(struct regulator_dev *rdev)
 {
 	int ret;
 
-	mutex_lock(&rdev->mutex);
+	if (!(rdev->flags & REGULATOR_UNSAFEOPS_FLAG))
+		mutex_lock(&rdev->mutex);
 
 	/* sanity check */
 	if (!rdev->desc->ops->get_current_limit) {
@@ -1710,7 +1752,8 @@ static int _regulator_get_current_limit(struct regulator_dev *rdev)
 
 	ret = rdev->desc->ops->get_current_limit(rdev);
 out:
-	mutex_unlock(&rdev->mutex);
+	if (!(rdev->flags & REGULATOR_UNSAFEOPS_FLAG))
+		mutex_unlock(&rdev->mutex);
 	return ret;
 }
 
@@ -1746,7 +1789,8 @@ int regulator_set_mode(struct regulator *regulator, unsigned int mode)
 	int ret;
 	int regulator_curr_mode;
 
-	mutex_lock(&rdev->mutex);
+	if (!(rdev->flags & REGULATOR_UNSAFEOPS_FLAG))
+		mutex_lock(&rdev->mutex);
 
 	/* sanity check */
 	if (!rdev->desc->ops->set_mode) {
@@ -1770,7 +1814,8 @@ int regulator_set_mode(struct regulator *regulator, unsigned int mode)
 
 	ret = rdev->desc->ops->set_mode(rdev, mode);
 out:
-	mutex_unlock(&rdev->mutex);
+	if (!(rdev->flags & REGULATOR_UNSAFEOPS_FLAG))
+		mutex_unlock(&rdev->mutex);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(regulator_set_mode);
@@ -1779,7 +1824,8 @@ static unsigned int _regulator_get_mode(struct regulator_dev *rdev)
 {
 	int ret;
 
-	mutex_lock(&rdev->mutex);
+	if (!(rdev->flags & REGULATOR_UNSAFEOPS_FLAG))
+		mutex_lock(&rdev->mutex);
 
 	/* sanity check */
 	if (!rdev->desc->ops->get_mode) {
@@ -1789,7 +1835,8 @@ static unsigned int _regulator_get_mode(struct regulator_dev *rdev)
 
 	ret = rdev->desc->ops->get_mode(rdev);
 out:
-	mutex_unlock(&rdev->mutex);
+	if (!(rdev->flags & REGULATOR_UNSAFEOPS_FLAG))
+		mutex_unlock(&rdev->mutex);
 	return ret;
 }
 
@@ -1838,7 +1885,8 @@ int regulator_set_optimum_mode(struct regulator *regulator, int uA_load)
 	int ret, output_uV, input_uV, total_uA_load = 0;
 	unsigned int mode;
 
-	mutex_lock(&rdev->mutex);
+	if (!(rdev->flags & REGULATOR_UNSAFEOPS_FLAG))
+		mutex_lock(&rdev->mutex);
 
 	regulator->uA_load = uA_load;
 	ret = regulator_check_drms(rdev);
@@ -1892,7 +1940,8 @@ int regulator_set_optimum_mode(struct regulator *regulator, int uA_load)
 	}
 	ret = mode;
 out:
-	mutex_unlock(&rdev->mutex);
+	if (!(rdev->flags & REGULATOR_UNSAFEOPS_FLAG))
+		mutex_unlock(&rdev->mutex);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(regulator_set_optimum_mode);
@@ -1934,6 +1983,9 @@ static void _notifier_call_chain(struct regulator_dev *rdev,
 				  unsigned long event, void *data)
 {
 	struct regulator_dev *_rdev;
+	/* We will not have notifiers in this mode */
+	if (rdev->flags & REGULATOR_UNSAFEOPS_FLAG)
+		return;
 
 	/* call rdev chain first */
 	blocking_notifier_call_chain(&rdev->notifier, event, NULL);
@@ -2423,6 +2475,9 @@ int regulator_suspend_prepare(suspend_state_t state)
 	/* ON is handled by regulator active state */
 	if (state == PM_SUSPEND_ON)
 		return -EINVAL;
+	/* we will not suspend if we are opened unsafe */
+	if (rdev->flags & REGULATOR_UNSAFEOPS_FLAG)
+		return -EINVAL;
 
 	mutex_lock(&regulator_list_mutex);
 	list_for_each_entry(rdev, &regulator_list, list) {
diff --git a/include/linux/regulator/consumer.h b/include/linux/regulator/consumer.h
index ebd7472..c071992 100644
--- a/include/linux/regulator/consumer.h
+++ b/include/linux/regulator/consumer.h
@@ -131,6 +131,8 @@ struct regulator *__must_check regulator_get(struct device *dev,
 					     const char *id);
 struct regulator *__must_check regulator_get_exclusive(struct device *dev,
 						       const char *id);
+struct regulator *__must_check regulator_get_exclusive_unsafe(struct device *dev,
+						       const char *id);
 void regulator_put(struct regulator *regulator);
 
 /* regulator output control and status */
diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index 6397ab3..6d117d9 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -157,6 +157,7 @@ struct regulator_desc {
 };
 
 #define REGULATOR_EXCLUSIVE_FLAG	(0x1 << 1)
+#define REGULATOR_UNSAFEOPS_FLAG	(0x2 << 1)
 
 /*
  * struct regulator_dev
-- 
1.6.3.3

Re: [PATCH 01/13] OMAP: Introduce a user list for each voltage domain instance in the voltage driver.

[Kevin Hilman]

Nishanth Menon <nm@ti.com> writes:

> Kevin Hilman had written, on 09/02/2010 12:47 PM, the following:

[...]

>>
>> If you look at the current PM branch (specifically pm-sr), you'll see
>> that with the updated SR layer, there is no SR enable/disable in the
>> idle path because there is no voltage scaling during idle.

[...]

>>
>> That being said, even if this is add later, the idle path can potentialy
>> call the SR API directly if needed for the enable/disable.
>
> not clean if it was directly implemented by regulator framework
>

The idea (so far) is that there will still be a low-level SmartReflex
API, and just a regulator "wrapper."  Thomas P. is experimenting with
extending the regulator framework to be able to call into external
low-level code which does the heavy lifting.  He can probably post an
RFC soon.

Kevin

Re: [PATCH 05/13] OMAP: Introduce device specific set rate and get rate in device opp structures.

[Kevin Hilman]

Thara Gopinath <thara@ti.com> writes:

> This patch extends the device opp structure to contain
> pointers to scale the operating rate of the
> device and to retrieve the operating rate of the device.
> This patch also adds the three new APIs in the opp layer
> namely opp_set_rate that can be called to set a new operating
> rate for a device, opp_get_rate that can be called to retrieve
> the operating frequency for a device and opp_populate_rate_fns
> to populte the device specific set_rate and get_rate API's.
> The opp_set_rate and opp_get_rate does some routine error
> checks and finally calls into the device specific set_rate
> and get_rate APIs populated through opp_populate_rate_fns.
>
> Signed-off-by: Thara Gopinath <thara@ti.com>

As I think about this more, I'm not sure the OPP layer is the right
layer for the get/set rate functions.  The OPP layer is currently just
the data store for OPP data. To me, these set/get functions are better
associated directly with an omap_device instead of an OPP.

For instance, the new OPP APIs are a bit confusing in terms of OPPs.
e.g: opp_get_rate().  What is the "rate" of an OPP, and what's the
difference with opp_get_freq()?

What's really happening is the rate is being changed for a device, and
the need for specific hooks are at the device level, not the OPP level.
For example, this current approach would not scale if you needed
multiple devices in the same domain that each needed custom
set_rate/get_rate hooks.

So instead, what about adding custom hooks at the omap_device level?
They could be registered at omap_device_build() time in the device
specific code.

Kevin

RE: [PATCH 01/13] OMAP: Introduce a user list for each voltage domain instance in the voltage driver.

[Gopinath, Thara]

>>-----Original Message-----
>>From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
>>Sent: Thursday, September 02, 2010 4:22 AM
>>To: Gopinath, Thara
>>Cc: linux-omap@vger.kernel.org; paul@pwsan.com; Sripathy, Vishwanath; Sawant, Anand; Cousson, Benoit;
>>thomas.petazzoni@free-electrons.com
>>Subject: Re: [PATCH 01/13] OMAP: Introduce a user list for each voltage domain instance in the
>>voltage driver.
>>
>>Thara Gopinath <thara@ti.com> writes:
>>
>>> This patch introduces a user list of devices associated with each
>>> voltage domain instance. The user list is implemented using plist
>>> structure with priority node populated with the voltage values.
>>> This patch also adds an API which will take in a device and
>>> requested voltage as parameters, adds the info to the user list
>>> and returns back the maximum voltage requested by all the user
>>> devices. This can be used anytime to get the voltage that the
>>> voltage domain instance can be transitioned into.
>>>
>>> Signed-off-by: Thara Gopinath <thara@ti.com>
>>
>>Looking closer at this, keeping track of a list of devices and
>>constraints is what the regulator framework does as well.
>>
>>Before we get too far down this path, we need to start working with
>>Thomas Petazzoni to better understand how we can use the regulator
>>framework for much of the management levels of the voltage layer.
>>
>>I'd rather not re-invent some of the management/constraints management
>>that could be done by the regulator framework.
>>
>>Basically, I think
>>
>>  r = regulator_get(dev, voltdm->name)
>>  regulator_set_voltage(r, volt)
>>
>>would basially be the equivalent of
>>
>>  omap_voltage_add_userreq(voldm, dev, &volt);
>>  omap_scale_voltage(voltdm, volt)

Hello Kevin,

Let me startr off by saying that I am not an expert on regulator
framework and hence my assessment below could be wrong.
I agree probably regulator framework would be 
the best place for this but then IMO regulator framework needs a lot of
changes to support the kind of use-counting we need in the voltage layer.
To start with regulator_get  keeps the use counting only for enabling or
disabling a regulator. It does not have a mechanism of adding the requested
voltage to a list and picking out the highest among the added values.
regulator_set_voltage could be used to route the call to omap_scale_voltage
but then it is just a wrapper.

Regards
Thara

Re: [PATCH 01/13] OMAP: Introduce a user list for each voltage domain instance in the voltage driver.

[Kevin Hilman]

+ Mark, Liam for regulator questions, thread in archives here:
  http://marc.info/?l=linux-omap&m=128349777617270&w=2

"Gopinath, Thara" <thara@ti.com> writes:

>>>-----Original Message-----
>>>From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
>>>Sent: Thursday, September 02, 2010 4:22 AM
>>>To: Gopinath, Thara
>>>Cc: linux-omap@vger.kernel.org; paul@pwsan.com; Sripathy, Vishwanath; Sawant, Anand; Cousson, Benoit;
>>>thomas.petazzoni@free-electrons.com
>>>Subject: Re: [PATCH 01/13] OMAP: Introduce a user list for each voltage domain instance in the
>>>voltage driver.
>>>
>>>Thara Gopinath <thara@ti.com> writes:
>>>
>>>> This patch introduces a user list of devices associated with each
>>>> voltage domain instance. The user list is implemented using plist
>>>> structure with priority node populated with the voltage values.
>>>> This patch also adds an API which will take in a device and
>>>> requested voltage as parameters, adds the info to the user list
>>>> and returns back the maximum voltage requested by all the user
>>>> devices. This can be used anytime to get the voltage that the
>>>> voltage domain instance can be transitioned into.
>>>>
>>>> Signed-off-by: Thara Gopinath <thara@ti.com>
>>>
>>>Looking closer at this, keeping track of a list of devices and
>>>constraints is what the regulator framework does as well.
>>>
>>>Before we get too far down this path, we need to start working with
>>>Thomas Petazzoni to better understand how we can use the regulator
>>>framework for much of the management levels of the voltage layer.
>>>
>>>I'd rather not re-invent some of the management/constraints management
>>>that could be done by the regulator framework.
>>>
>>>Basically, I think
>>>
>>>  r = regulator_get(dev, voltdm->name)
>>>  regulator_set_voltage(r, volt)
>>>
>>>would basially be the equivalent of
>>>
>>>  omap_voltage_add_userreq(voldm, dev, &volt);
>>>  omap_scale_voltage(voltdm, volt)
>
> Hello Kevin,
>
> Let me startr off by saying that I am not an expert on regulator
> framework and hence my assessment below could be wrong.
> I agree probably regulator framework would be 
> the best place for this but then IMO regulator framework needs a lot of
> changes to support the kind of use-counting we need in the voltage layer.
> To start with regulator_get  keeps the use counting only for enabling or
> disabling a regulator. It does not have a mechanism of adding the requested
> voltage to a list and picking out the highest among the added values.

Like you, I'm no expert on the regulator framework internals, but it
appears to have a pretty thorough system of constraints management that
upon first glance seems to be a good fit for what we need.  It may need
to be extended, but I would rather see us enhance the regulator
framework than re-invent the constraints management.

> regulator_set_voltage could be used to route the call to omap_scale_voltage
> but then it is just a wrapper.

Yes, just a wrapper, but users would be then using a a well defined and
well documented API.

Kevin

Re: [PATCH 01/13] OMAP: Introduce a user list for each voltage domain instance in the voltage driver.

[Mark Brown]

On Fri, Sep 03, 2010 at 09:41:11AM -0700, Kevin Hilman wrote:

> Like you, I'm no expert on the regulator framework internals, but it
> appears to have a pretty thorough system of constraints management that
> upon first glance seems to be a good fit for what we need.  It may need
> to be extended, but I would rather see us enhance the regulator
> framework than re-invent the constraints management.

This seems reasonable, the only thing I'm wary of with this stuff is
adding things to manage anything outside voltages since I'm not
convinced that the requirements of different processors for other things
are sufficiently well understood to make a simple abstraction.  

The only thing I can think you might need to do if this is just straight
voltage setting is re-add support for multiple consumers setting
voltages simultaneously (there was someone from Qualcomm talking about
that as well but I didn't see any patches from him) but the API side is
all there and the core support ought to be relatively straightforward.

Re: [PATCH 01/13] OMAP: Introduce a user list for each voltage domain instance in the voltage driver.

[Kevin Hilman]

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

> On Fri, Sep 03, 2010 at 09:41:11AM -0700, Kevin Hilman wrote:
>
>> Like you, I'm no expert on the regulator framework internals, but it
>> appears to have a pretty thorough system of constraints management that
>> upon first glance seems to be a good fit for what we need.  It may need
>> to be extended, but I would rather see us enhance the regulator
>> framework than re-invent the constraints management.
>
> This seems reasonable, the only thing I'm wary of with this stuff is
> adding things to manage anything outside voltages since I'm not
> convinced that the requirements of different processors for other things
> are sufficiently well understood to make a simple abstraction.  

Mark, thanks for your input.

For now, at least on OMAP, we're only thinking of managing voltages for
the primary voltage rails on chip.

We already have well defined layers for managing our power domains and
clockdomains and prefer to keep that as OMAP-internal code, as it's
nothing that drivers really need to be aware of.

> The only thing I can think you might need to do if this is just straight
> voltage setting is re-add support for multiple consumers setting
> voltages simultaneously 

Yeah, that sounds like what we need.

re-add? was it there at one point and removed?  Any pointers to the old
code?

> (there was someone from Qualcomm talking about
> that as well but I didn't see any patches from him) but the API side is
> all there and the core support ought to be relatively straightforward.

OK, thanks for the feedback.

Kevin

Re: [PATCH 01/13] OMAP: Introduce a user list for each voltage domain instance in the voltage driver.

[Mark Brown]

On Fri, Sep 03, 2010 at 11:00:31AM -0700, Kevin Hilman wrote:
> Mark Brown <broonie@opensource.wolfsonmicro.com> writes:

> > The only thing I can think you might need to do if this is just straight
> > voltage setting is re-add support for multiple consumers setting
> > voltages simultaneously 

> Yeah, that sounds like what we need.

> re-add? was it there at one point and removed?  Any pointers to the old
> code?

It was present in the pre-merge regulator API which you can find in the
bowels of:

   git://opensource.wolfsonmicro.com/linux-2.6-audioplus

but was removed to simplify review during the merge.  It's fairly simple
to do, it's just that there's been no demand.

Essentially all that needs doing is that when regulator_set_voltage() is
called instead of merging with the machine constraints and applying the
setting immediately we store the constraints that are specified in the
consumer then iterate over all enabled consumers applying all the
constraints that they've set in addition to those from the machine.
This results in a configuration which is the lowest possible voltage
which satisfies all the constraints that have been supplied and for
supplies with only one consumer it gives the same behaviour as we have
currently.

Re: [PATCH 01/13] OMAP: Introduce a user list for each voltage domain instance in the voltage driver.

[Kevin Hilman]

"Gopinath, Thara" <thara@ti.com> writes:

[...]

> I agree probably regulator framework would be 
> the best place for this but then IMO regulator framework needs a lot of
> changes to support the kind of use-counting we need in the voltage layer.
> To start with regulator_get  keeps the use counting only for enabling or
> disabling a regulator. It does not have a mechanism of adding the requested
> voltage to a list and picking out the highest among the added values.
> regulator_set_voltage could be used to route the call to omap_scale_voltage
> but then it is just a wrapper.

Hi Thara,

Based on feedback from regulator folks (and also remembering that we
agreed to use the regulator framework as a later phase) let's keep your
current approach as an interim solution, but knowing that we will
eventually switch to use the regulator framework for handling multiple
requests.

In the background Thomas and I will continue to research the changes
needed in the regulator framework.

Thanks,

Kevin

Re: [PATCH 01/13] OMAP: Introduce a user list for each voltage domain instance in the voltage driver.

[Mark Brown]

On Fri, Sep 03, 2010 at 11:27:50AM -0700, Kevin Hilman wrote:

> In the background Thomas and I will continue to research the changes
> needed in the regulator framework.

These really are fairly straightforward, the algorithm I outlined in my
last mail will do the trick.

Re: [PATCH 01/13] OMAP: Introduce a user list for each voltage domain instance in the voltage driver.

[Eduardo Valentin]

Hello,

On Fri, Sep 03, 2010 at 08:20:52PM +0200, Mark Brown wrote:
> On Fri, Sep 03, 2010 at 11:00:31AM -0700, Kevin Hilman wrote:
> > Mark Brown <broonie@opensource.wolfsonmicro.com> writes:
> 
> > > The only thing I can think you might need to do if this is just straight
> > > voltage setting is re-add support for multiple consumers setting
> > > voltages simultaneously 
> 
> > Yeah, that sounds like what we need.
> 
> > re-add? was it there at one point and removed?  Any pointers to the old
> > code?
> 
> It was present in the pre-merge regulator API which you can find in the
> bowels of:
> 
>    git://opensource.wolfsonmicro.com/linux-2.6-audioplus
> 
> but was removed to simplify review during the merge.  It's fairly simple
> to do, it's just that there's been no demand.
> 
> Essentially all that needs doing is that when regulator_set_voltage() is
> called instead of merging with the machine constraints and applying the
> setting immediately we store the constraints that are specified in the
> consumer then iterate over all enabled consumers applying all the
> constraints that they've set in addition to those from the machine.
> This results in a configuration which is the lowest possible voltage
> which satisfies all the constraints that have been supplied and for
> supplies with only one consumer it gives the same behaviour as we have
> currently.

How about taking Thara's proposal of using priority lists?

I mean, it could make more sense to keep the constraints into a priority list,
instead of "iterate over all enabled consumers"?

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

---
Eduardo Valentin

Re: [PATCH 01/13] OMAP: Introduce a user list for each voltage domain instance in the voltage driver.

[Liam Girdwood]

On Mon, 2010-09-06 at 22:59 +0300, Eduardo Valentin wrote:
> Hello,
> 
> On Fri, Sep 03, 2010 at 08:20:52PM +0200, Mark Brown wrote:
> > On Fri, Sep 03, 2010 at 11:00:31AM -0700, Kevin Hilman wrote:
> > > Mark Brown <broonie@opensource.wolfsonmicro.com> writes:
> > 
> > > > The only thing I can think you might need to do if this is just straight
> > > > voltage setting is re-add support for multiple consumers setting
> > > > voltages simultaneously 
> > 
> > > Yeah, that sounds like what we need.
> > 
> > > re-add? was it there at one point and removed?  Any pointers to the old
> > > code?
> > 
> > It was present in the pre-merge regulator API which you can find in the
> > bowels of:
> > 
> >    git://opensource.wolfsonmicro.com/linux-2.6-audioplus
> > 
> > but was removed to simplify review during the merge.  It's fairly simple
> > to do, it's just that there's been no demand.
> > 
> > Essentially all that needs doing is that when regulator_set_voltage() is
> > called instead of merging with the machine constraints and applying the
> > setting immediately we store the constraints that are specified in the
> > consumer then iterate over all enabled consumers applying all the
> > constraints that they've set in addition to those from the machine.
> > This results in a configuration which is the lowest possible voltage
> > which satisfies all the constraints that have been supplied and for
> > supplies with only one consumer it gives the same behaviour as we have
> > currently.
> 
> How about taking Thara's proposal of using priority lists?
> 
> I mean, it could make more sense to keep the constraints into a priority list,
> instead of "iterate over all enabled consumers"?
> 

You've not really explained why you think a priority list makes more
sense in this case ?

Liam
-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

Re: [PATCH 01/13] OMAP: Introduce a user list for each voltage domain instance in the voltage driver.

[Mark Brown]

On Mon, Sep 06, 2010 at 10:59:05PM +0300, Eduardo Valentin wrote:
> On Fri, Sep 03, 2010 at 08:20:52PM +0200, Mark Brown wrote:

> > Essentially all that needs doing is that when regulator_set_voltage() is
> > called instead of merging with the machine constraints and applying the
> > setting immediately we store the constraints that are specified in the
> > consumer then iterate over all enabled consumers applying all the
> > constraints that they've set in addition to those from the machine.

> How about taking Thara's proposal of using priority lists?

> I mean, it could make more sense to keep the constraints into a priority list,
> instead of "iterate over all enabled consumers"?

Partly just that the lists tend to be short enough that it's hardly
worth bothering with the more complex algorithm, partly because the fact
that you have both a minimum constraint and a maximum constraint makes
it into two priority lists you need to maintain.

RE: [PATCH 01/13] OMAP: Introduce a user list for each voltage domain instance in the voltage driver.

[Gopinath, Thara]

>>-----Original Message-----
>>From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
>>Sent: Saturday, August 28, 2010 5:23 AM
>>To: Gopinath, Thara
>>Cc: linux-omap@vger.kernel.org; paul@pwsan.com; Sripathy, Vishwanath; Sawant, Anand; Cousson, Benoit
>>Subject: Re: [PATCH 01/13] OMAP: Introduce a user list for each voltage domain instance in the
>>voltage driver.
>>
>>Thara Gopinath <thara@ti.com> writes:
>>
>>> This patch introduces a user list of devices associated with each
>>> voltage domain instance. The user list is implemented using plist
>>> structure with priority node populated with the voltage values.
>>> This patch also adds an API which will take in a device and
>>> requested voltage as parameters, adds the info to the user list
>>> and returns back the maximum voltage requested by all the user
>>> devices. This can be used anytime to get the voltage that the
>>> voltage domain instance can be transitioned into.
>>>
>>> Signed-off-by: Thara Gopinath <thara@ti.com>
>>> ---
>>>  arch/arm/mach-omap2/voltage.c             |   94 +++++++++++++++++++++++++++++
>>>  arch/arm/plat-omap/include/plat/voltage.h |    2 +
>>>  2 files changed, 96 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/voltage.c b/arch/arm/mach-omap2/voltage.c
>>> index 6a07fe9..4624250 100644
>>> --- a/arch/arm/mach-omap2/voltage.c
>>> +++ b/arch/arm/mach-omap2/voltage.c
>>> @@ -24,6 +24,9 @@
>>>  #include <linux/clk.h>
>>>  #include <linux/err.h>
>>>  #include <linux/debugfs.h>
>>> +#include <linux/spinlock.h>
>>> +#include <linux/plist.h>
>>> +#include <linux/slab.h>
>>>
>>>  #include <plat/omap-pm.h>
>>>  #include <plat/omap34xx.h>
>>> @@ -95,6 +98,20 @@ struct vp_reg_val {
>>>  };
>>>
>>>  /**
>>> + * omap_vdd_user_list	- The per vdd user list
>>> + *
>>> + * @dev		: The device asking for the vdd to be set at a particular
>>> + *		  voltage
>>> + * @node	: The list head entry
>>> + * @volt	: The voltage requested by the device <dev>
>>> + */
>>> +struct omap_vdd_user_list {
>>> +	struct device *dev;
>>> +	struct plist_node node;
>>> +	u32 volt;
>>> +};
>>> +
>>> +/**
>>>   * omap_vdd_info - Per Voltage Domain info
>>>   *
>>>   * @volt_data		: voltage table having the distinct voltages supported
>>> @@ -105,6 +122,9 @@ struct vp_reg_val {
>>>   *			  vp registers
>>>   * @volt_clk		: the clock associated with the vdd.
>>>   * @opp_dev		: the 'struct device' associated with this vdd.
>>> + * @user_lock		: the lock to be used by the plist user_list
>>> + * @user_list		: the list head maintaining the various users
>>> + *			  of this vdd with the voltage requested by each user.
>>>   * @volt_data_count	: Number of distinct voltages supported by this vdd.
>>>   * @nominal_volt	: Nominal voltaged for this vdd.
>>>   * cmdval_reg		: Voltage controller cmdval register.
>>> @@ -117,6 +137,9 @@ struct omap_vdd_info{
>>>  	struct clk *volt_clk;
>>>  	struct device *opp_dev;
>>>  	struct voltagedomain voltdm;
>>> +	spinlock_t user_lock;
>>> +	struct plist_head user_list;
>>> +	struct mutex scaling_mutex;
>>>  	int volt_data_count;
>>>  	unsigned long nominal_volt;
>>>  	u8 cmdval_reg;
>>> @@ -785,11 +808,18 @@ static void __init vdd_data_configure(struct omap_vdd_info *vdd)
>>>  	struct dentry *vdd_debug;
>>>  	char name[16];
>>>  #endif
>>> +
>>>  	if (cpu_is_omap34xx())
>>>  		omap3_vdd_data_configure(vdd);
>>>  	else if (cpu_is_omap44xx())
>>>  		omap4_vdd_data_configure(vdd);
>>>
>>> +	/* Init the plist */
>>> +	spin_lock_init(&vdd->user_lock);
>>> +	plist_head_init(&vdd->user_list, &vdd->user_lock);
>>> +	/* Init the DVFS mutex */
>>> +	mutex_init(&vdd->scaling_mutex);
>>> +
>>>  #ifdef CONFIG_PM_DEBUG
>>>  	strcpy(name, "vdd_");
>>>  	strcat(name, vdd->voltdm.name);
>>> @@ -1142,6 +1172,70 @@ unsigned long omap_vp_get_curr_volt(struct voltagedomain *voltdm)
>>>  }
>>>
>>>  /**
>>> + * omap_voltage_add_userreq : API to keep track of various requests to
>>> + *			    scale the VDD and returns the best possible
>>> + *			    voltage the VDD can be put to.
>>> + * @volt_domain: pointer to the voltage domain.
>>> + * @dev : the device pointer.
>>> + * @volt : the voltage which is requested by the device.
>>> + *
>>> + * This API is to be called before the actual voltage scaling is
>>> + * done to determine what is the best possible voltage the VDD can
>>> + * be put to. This API adds the device <dev> in the user list of the
>>> + * vdd <volt_domain> with <volt> as the requested voltage. The user list
>>> + * is a plist with the priority element absolute voltage values.
>>> + * The API then finds the maximum of all the requested voltages for
>>> + * the VDD and returns it back through <volt> pointer itself.
>>> + * Returns error value in case of any errors.
>>> + */
>>> +int omap_voltage_add_userreq(struct voltagedomain *voltdm, struct device *dev,
>>> +		unsigned long *volt)
>>
>>How about just omap_voltage_add_request()
>>
>>Also, do we need both voltdm and dev?  With your other patches, can't we
>>look up the voltm based on dev?  Or, is there a need for a device to add
>>a request in a voltage domain other than the one to which it belongs?
>>
>>Also, how does one remove a voltage request?

Hello Kevin,

I could rename this API to what you have suggested. Yes we do need voltdm and dev.
Let us say I have three devices in a voltdm and I need to maintain a request for each
of these devices.

When a omap_device_set_rate API is called by the device to lower its rate the voltage
request will automatically get lowered.

>>
>>> +{
>>> +	struct omap_vdd_info *vdd;
>>> +	struct omap_vdd_user_list *user;
>>> +	struct plist_node *node;
>>> +	int found = 0;
>>> +
>>> +	if (!voltdm || IS_ERR(voltdm)) {
>>> +		pr_warning("%s: VDD specified does not exist!\n", __func__);
>>> +		return -EINVAL;
>>> +	}
>>> +
>>> +	vdd = container_of(voltdm, struct omap_vdd_info, voltdm);
>>> +
>>> +	mutex_lock(&vdd->scaling_mutex);
>>> +
>>> +	plist_for_each_entry(user, &vdd->user_list, node) {
>>> +		if (user->dev == dev) {
>>> +			found = 1;
>>> +			break;
>>> +		}
>>> +	}
>>
>>Minor: I'm not crazy about the 'found' flag.  IMO, readability is
>>improved if you init 'user = NULL' above, use a tmp pointer to
>>walk the list, and rather than 'found = 1', do 'user = tmp_user'
>>when you find the match.

Ok will do.

>>
>>> +
>>> +	if (!found) {
>>
>>and here, do if (!user)
>>
>>> +		user = kzalloc(sizeof(struct omap_vdd_user_list), GFP_KERNEL);
>>> +		if (!user) {
>>> +			pr_err("%s: Unable to creat a new user for vdd_%s\n",
>>> +				__func__, voltdm->name);
>>> +			mutex_unlock(&vdd->scaling_mutex);
>>> +			return -ENOMEM;
>>> +		}
>>> +		user->dev = dev;
>>> +	} else {
>>> +		plist_del(&user->node, &vdd->user_list);
>>
>>hmm... if we get here, we didn't find a match, so 'user' points to the
>>last element in the list (which is the highest voltage request, I
>>guess), or even NULL if the list is empty.  Then, this node is deleted.
>>
>>-ECONFUSED

I do not understand this comment. But I will surely look into this part of code.
Btw this code is now more tested out with OMAP4 internally.

>>
>>> +	}
>>> +
>>> +	plist_node_init(&user->node, *volt);
>>> +	plist_add(&user->node, &vdd->user_list);
>>> +	node = plist_first(&vdd->user_list);
>>> +	*volt = node->prio;
>>> +
>>> +	mutex_unlock(&vdd->scaling_mutex);
>>> +
>>> +	return 0;
>>> +}
>>
>>Can't think of a use-case now (cuz it's Friday), but would we ever want
>>to add multiple requests per device?
>>
>>The current approch will only track one request per device.
IMHO only as of now only one request per device needs to be tracked. A device the volt
domain to be at only one particular voltage at any instance. 

Regards
Thara

RE: [PATCH 04/13] OMAP: Introduce API to return a device list associated with a voltage domain

[Gopinath, Thara]

>>-----Original Message-----
>>From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
>>Sent: Saturday, August 28, 2010 6:23 AM
>>To: Gopinath, Thara
>>Cc: linux-omap@vger.kernel.org; paul@pwsan.com; Sripathy, Vishwanath; Sawant, Anand; Cousson, Benoit
>>Subject: Re: [PATCH 04/13] OMAP: Introduce API to return a device list associated with a voltage
>>domain
>>
>>Thara Gopinath <thara@ti.com> writes:
>>
>>> This patch adds an API in the opp layer that
>>> can be used by the voltage layer to get a list of all the
>>> scalable devices belonging to a particular voltage domain.
>>> This API is to be typically called only once by the voltage
>>> layer per voltage domain instance and the device list should
>>> be stored. This approach makes it easy during dvfs to scale
>>> all the devices associated with a voltage domain and then
>>> scale the voltage domain.
>>>
>>> Signed-off-by: Thara Gopinath <thara@ti.com>
>>
>>I think this should be done in two steps.
>>
>>During init, each OPP
>>
>>> ---
>>>  arch/arm/plat-omap/include/plat/opp.h |    9 +++++++++
>>>  arch/arm/plat-omap/opp.c              |   28 ++++++++++++++++++++++++++++
>>>  2 files changed, 37 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/arch/arm/plat-omap/include/plat/opp.h b/arch/arm/plat-omap/include/plat/opp.h
>>> index 0e580ed..a4c1669 100644
>>> --- a/arch/arm/plat-omap/include/plat/opp.h
>>> +++ b/arch/arm/plat-omap/include/plat/opp.h
>>> @@ -18,6 +18,7 @@
>>>  #include <linux/cpufreq.h>
>>>
>>>  #include <plat/common.h>
>>> +#include <plat/voltage.h>
>>>
>>>  /**
>>>   * struct omap_opp_def - OMAP OPP Definition
>>> @@ -86,6 +87,9 @@ int opp_disable(struct omap_opp *opp);
>>>
>>>  void opp_init_cpufreq_table(struct device *dev,
>>>  			    struct cpufreq_frequency_table **table);
>>> +
>>> +struct device **opp_init_voltage_params(struct voltagedomain *voltdm,
>>> +					int *dev_count);
>>>  #else
>>>  static inline unsigned long opp_get_voltage(const struct omap_opp *opp)
>>>  {
>>> @@ -149,5 +153,10 @@ void opp_init_cpufreq_table(struct omap_opp *opps,
>>>  {
>>>  }
>>>
>>> +static inline struct device **opp_init_voltage_params(
>>> +			struct voltagedomain *voltdm, int *dev_count)
>>> +{
>>> +}
>>> +
>>>  #endif		/* CONFIG_PM */
>>>  #endif		/* __ASM_ARM_OMAP_OPP_H */
>>> diff --git a/arch/arm/plat-omap/opp.c b/arch/arm/plat-omap/opp.c
>>> index a3dea82..72dd62a 100644
>>> --- a/arch/arm/plat-omap/opp.c
>>> +++ b/arch/arm/plat-omap/opp.c
>>> @@ -502,3 +502,31 @@ void opp_init_cpufreq_table(struct device *dev,
>>>
>>>  	*table = &freq_table[0];
>>>  }
>>> +
>>> +struct device **opp_init_voltage_params(struct voltagedomain *voltdm,
>>> +					int *dev_count)
>>> +{
>>> +	struct device_opp *dev_opp;
>>> +	struct device **dev_list;
>>> +	int count = 0, i = 0;
>>> +
>>> +	list_for_each_entry(dev_opp, &dev_opp_list, node) {
>>> +		if (!dev_opp->oh->vdd_name)
>>> +			continue;
>>> +
>>> +		if (!strcmp(dev_opp->oh->vdd_name, voltdm->name)) {
>>> +			dev_opp->oh->voltdm = voltdm;
>>
>>Couldn't we assign the voltdm at opp_add() time since you added it as
>>part of the hwmod?

We cannot as the voltage layer is not initialized at the point of opp_add.
Having said this, today voltage layer is dependent on opp layer only to figure out 
the current nominal voltage from the opp table. If that can be some how decoupled we
can initialize voltage layer early on and implement this.

Regards
Thara

RE: [PATCH 04/13] OMAP: Introduce API to return a device list associated with a voltage domain

[Gopinath, Thara]

>>-----Original Message-----
>>From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
>>Sent: Thursday, September 02, 2010 6:04 AM
>>To: Gopinath, Thara
>>Cc: linux-omap@vger.kernel.org; paul@pwsan.com; Sripathy, Vishwanath; Sawant, Anand; Cousson, Benoit
>>Subject: Re: [PATCH 04/13] OMAP: Introduce API to return a device list associated with a voltage
>>domain
>>
>>Thara Gopinath <thara@ti.com> writes:
>>
>>> This patch adds an API in the opp layer that
>>> can be used by the voltage layer to get a list of all the
>>> scalable devices belonging to a particular voltage domain.
>>> This API is to be typically called only once by the voltage
>>> layer per voltage domain instance and the device list should
>>> be stored. This approach makes it easy during dvfs to scale
>>> all the devices associated with a voltage domain and then
>>> scale the voltage domain.
>>>
>>> Signed-off-by: Thara Gopinath <thara@ti.com>
>>
>>I don't think the OPP layer is the right place for this after all.
>>
>>How about something like this in the voltage layer:
>>
>>  omap_voltage_add_device(struct voltagedomain *voltdm, struct device *dev)
>>
>>During omap_device_build(), if the hwmod has a voltage domain, it
>>calls this function to register it with the voltage layer.

This mandates voltage layer to be initialized before the first omap_device_build
has happened. I think that is going to be a very confusing sequencing. Also today
voltage layer init happens later in the system.

>>
>>This function then creates a list (internal to voltage layer) of all the
>>devices in a voltage domain rather than having to query the OPP layer
>>for it.
>>
>>Kevin
>>
>>> ---
>>>  arch/arm/plat-omap/include/plat/opp.h |    9 +++++++++
>>>  arch/arm/plat-omap/opp.c              |   28 ++++++++++++++++++++++++++++
>>>  2 files changed, 37 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/arch/arm/plat-omap/include/plat/opp.h b/arch/arm/plat-omap/include/plat/opp.h
>>> index 0e580ed..a4c1669 100644
>>> --- a/arch/arm/plat-omap/include/plat/opp.h
>>> +++ b/arch/arm/plat-omap/include/plat/opp.h
>>> @@ -18,6 +18,7 @@
>>>  #include <linux/cpufreq.h>
>>>
>>>  #include <plat/common.h>
>>> +#include <plat/voltage.h>
>>>
>>>  /**
>>>   * struct omap_opp_def - OMAP OPP Definition
>>> @@ -86,6 +87,9 @@ int opp_disable(struct omap_opp *opp);
>>>
>>>  void opp_init_cpufreq_table(struct device *dev,
>>>  			    struct cpufreq_frequency_table **table);
>>> +
>>> +struct device **opp_init_voltage_params(struct voltagedomain *voltdm,
>>> +					int *dev_count);
>>>  #else
>>>  static inline unsigned long opp_get_voltage(const struct omap_opp *opp)
>>>  {
>>> @@ -149,5 +153,10 @@ void opp_init_cpufreq_table(struct omap_opp *opps,
>>>  {
>>>  }
>>>
>>> +static inline struct device **opp_init_voltage_params(
>>> +			struct voltagedomain *voltdm, int *dev_count)
>>> +{
>>> +}
>>> +
>>>  #endif		/* CONFIG_PM */
>>>  #endif		/* __ASM_ARM_OMAP_OPP_H */
>>> diff --git a/arch/arm/plat-omap/opp.c b/arch/arm/plat-omap/opp.c
>>> index a3dea82..72dd62a 100644
>>> --- a/arch/arm/plat-omap/opp.c
>>> +++ b/arch/arm/plat-omap/opp.c
>>> @@ -502,3 +502,31 @@ void opp_init_cpufreq_table(struct device *dev,
>>>
>>>  	*table = &freq_table[0];
>>>  }
>>> +
>>> +struct device **opp_init_voltage_params(struct voltagedomain *voltdm,
>>> +					int *dev_count)
>>> +{
>>> +	struct device_opp *dev_opp;
>>> +	struct device **dev_list;
>>> +	int count = 0, i = 0;
>>> +
>>> +	list_for_each_entry(dev_opp, &dev_opp_list, node) {
>>> +		if (!dev_opp->oh->vdd_name)
>>> +			continue;
>>> +
>>> +		if (!strcmp(dev_opp->oh->vdd_name, voltdm->name)) {
>>> +			dev_opp->oh->voltdm = voltdm;
>>> +			count++;
>>> +		}
>>> +	}
>>> +
>>> +	dev_list = kzalloc(sizeof(struct device *) * count, GFP_KERNEL);
>>> +
>>> +	list_for_each_entry(dev_opp, &dev_opp_list, node) {
>>> +		if (dev_opp->oh->voltdm == voltdm)
>>> +			dev_list[i++] = dev_opp->dev;
>>> +	}
>>> +
>>> +	*dev_count = count;
>>> +	return dev_list;
>>> +}

RE: [PATCH 05/13] OMAP: Introduce device specific set rate and get rate in device opp structures.

[Gopinath, Thara]

>>-----Original Message-----
>>From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
>>Sent: Friday, September 03, 2010 5:12 AM
>>To: Gopinath, Thara
>>Cc: linux-omap@vger.kernel.org; paul@pwsan.com; Sripathy, Vishwanath; Sawant, Anand; Cousson, Benoit
>>Subject: Re: [PATCH 05/13] OMAP: Introduce device specific set rate and get rate in device opp
>>structures.
>>
>>Thara Gopinath <thara@ti.com> writes:
>>
>>> This patch extends the device opp structure to contain
>>> pointers to scale the operating rate of the
>>> device and to retrieve the operating rate of the device.
>>> This patch also adds the three new APIs in the opp layer
>>> namely opp_set_rate that can be called to set a new operating
>>> rate for a device, opp_get_rate that can be called to retrieve
>>> the operating frequency for a device and opp_populate_rate_fns
>>> to populte the device specific set_rate and get_rate API's.
>>> The opp_set_rate and opp_get_rate does some routine error
>>> checks and finally calls into the device specific set_rate
>>> and get_rate APIs populated through opp_populate_rate_fns.
>>>
>>> Signed-off-by: Thara Gopinath <thara@ti.com>
>>
>>As I think about this more, I'm not sure the OPP layer is the right
>>layer for the get/set rate functions.  The OPP layer is currently just
>>the data store for OPP data. To me, these set/get functions are better
>>associated directly with an omap_device instead of an OPP.
>>
>>For instance, the new OPP APIs are a bit confusing in terms of OPPs.
>>e.g: opp_get_rate().  What is the "rate" of an OPP, and what's the
>>difference with opp_get_freq()?
>>
>>What's really happening is the rate is being changed for a device, and
>>the need for specific hooks are at the device level, not the OPP level.
>>For example, this current approach would not scale if you needed
>>multiple devices in the same domain that each needed custom
>>set_rate/get_rate hooks.
>>
>>So instead, what about adding custom hooks at the omap_device level?
>>They could be registered at omap_device_build() time in the device
>>specific code.

This is exactly what I had in my mind when I started implementing this.
But then Paul said hwmod or omap_device is not the place to keep it. Also I do not
want the set rate get rate APIs for devices that require changes dividers in the PRCM
clock module to be spread out in the system. Makes it very very difficult to debug.
If we agree to add the set_rate and get_rate in the omap_device structure, I would like to have
one more API in the omap_device layer to register these APIs device wise. Thus we can keep these
set rate and get rate APIs in one single file.

Also if we move these hooks to omap_device struct we will need to rename the current omap_device_set_rate (the main API) to omap_device_scale and introduce a new omap_device_set_rate which just finds out the omap device from the passed dev pointer and does a od->set_rate. Similarly for get rate also.

Regards
Thara

Re: [PATCH 01/13] OMAP: Introduce a user list for each voltage domain instance in the voltage driver.

[Kevin Hilman]

"Gopinath, Thara" <thara@ti.com> writes:

[...]

>>>> + * omap_voltage_add_userreq : API to keep track of various requests to
>>>> + *			    scale the VDD and returns the best possible
>>>> + *			    voltage the VDD can be put to.
>>>> + * @volt_domain: pointer to the voltage domain.
>>>> + * @dev : the device pointer.
>>>> + * @volt : the voltage which is requested by the device.
>>>> + *
>>>> + * This API is to be called before the actual voltage scaling is
>>>> + * done to determine what is the best possible voltage the VDD can
>>>> + * be put to. This API adds the device <dev> in the user list of the
>>>> + * vdd <volt_domain> with <volt> as the requested voltage. The user list
>>>> + * is a plist with the priority element absolute voltage values.
>>>> + * The API then finds the maximum of all the requested voltages for
>>>> + * the VDD and returns it back through <volt> pointer itself.
>>>> + * Returns error value in case of any errors.
>>>> + */
>>>> +int omap_voltage_add_userreq(struct voltagedomain *voltdm, struct device *dev,
>>>> +		unsigned long *volt)
>>>
>>>How about just omap_voltage_add_request()
>>>
>>>Also, do we need both voltdm and dev?  With your other patches, can't we
>>>look up the voltm based on dev?  Or, is there a need for a device to add
>>>a request in a voltage domain other than the one to which it belongs?
>>>
>>>Also, how does one remove a voltage request?
>
> Hello Kevin,
>
> I could rename this API to what you have suggested. Yes we do need voltdm and dev.
> Let us say I have three devices in a voltdm and I need to maintain a request for each
> of these devices.

OK, thanks for clarifying.

> When a omap_device_set_rate API is called by the device to lower its rate the voltage
> request will automatically get lowered.

OK, but what about removing a request when a device no longer has any
voltage constraints. 

[...]

Kevin

Re: [PATCH 04/13] OMAP: Introduce API to return a device list associated with a voltage domain

[Kevin Hilman]

"Gopinath, Thara" <thara@ti.com> writes:

[...]

>>>> +struct device **opp_init_voltage_params(struct voltagedomain *voltdm,
>>>> +					int *dev_count)
>>>> +{
>>>> +	struct device_opp *dev_opp;
>>>> +	struct device **dev_list;
>>>> +	int count = 0, i = 0;
>>>> +
>>>> +	list_for_each_entry(dev_opp, &dev_opp_list, node) {
>>>> +		if (!dev_opp->oh->vdd_name)
>>>> +			continue;
>>>> +
>>>> +		if (!strcmp(dev_opp->oh->vdd_name, voltdm->name)) {
>>>> +			dev_opp->oh->voltdm = voltdm;
>>>
>>>Couldn't we assign the voltdm at opp_add() time since you added it as
>>>part of the hwmod?
>
> We cannot as the voltage layer is not initialized at the point of opp_add.
> Having said this, today voltage layer is dependent on opp layer only to figure out 
> the current nominal voltage from the opp table. If that can be some how decoupled we
> can initialize voltage layer early on and implement this.

We could decouple the voltage init into and early init and late init to
handle this.

Kevin

Re: [PATCH 04/13] OMAP: Introduce API to return a device list associated with a voltage domain

[Kevin Hilman]

"Gopinath, Thara" <thara@ti.com> writes:

>>>-----Original Message-----
>>>From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
>>>Sent: Thursday, September 02, 2010 6:04 AM
>>>To: Gopinath, Thara
>>>Cc: linux-omap@vger.kernel.org; paul@pwsan.com; Sripathy, Vishwanath; Sawant, Anand; Cousson, Benoit
>>>Subject: Re: [PATCH 04/13] OMAP: Introduce API to return a device list associated with a voltage
>>>domain
>>>
>>>Thara Gopinath <thara@ti.com> writes:
>>>
>>>> This patch adds an API in the opp layer that
>>>> can be used by the voltage layer to get a list of all the
>>>> scalable devices belonging to a particular voltage domain.
>>>> This API is to be typically called only once by the voltage
>>>> layer per voltage domain instance and the device list should
>>>> be stored. This approach makes it easy during dvfs to scale
>>>> all the devices associated with a voltage domain and then
>>>> scale the voltage domain.
>>>>
>>>> Signed-off-by: Thara Gopinath <thara@ti.com>
>>>
>>>I don't think the OPP layer is the right place for this after all.
>>>
>>>How about something like this in the voltage layer:
>>>
>>>  omap_voltage_add_device(struct voltagedomain *voltdm, struct device *dev)
>>>
>>>During omap_device_build(), if the hwmod has a voltage domain, it
>>>calls this function to register it with the voltage layer.
>
> This mandates voltage layer to be initialized before the first omap_device_build
> has happened. I think that is going to be a very confusing sequencing. Also today
> voltage layer init happens later in the system.

If we split into an early and late init, that would help here too.

Kevin

Re: [PATCH 05/13] OMAP: Introduce device specific set rate and get rate in device opp structures.

[Kevin Hilman]

"Gopinath, Thara" <thara@ti.com> writes:

>>>-----Original Message-----
>>>From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
>>>Sent: Friday, September 03, 2010 5:12 AM
>>>To: Gopinath, Thara
>>>Cc: linux-omap@vger.kernel.org; paul@pwsan.com; Sripathy, Vishwanath; Sawant, Anand; Cousson, Benoit
>>>Subject: Re: [PATCH 05/13] OMAP: Introduce device specific set rate and get rate in device opp
>>>structures.
>>>
>>>Thara Gopinath <thara@ti.com> writes:
>>>
>>>> This patch extends the device opp structure to contain
>>>> pointers to scale the operating rate of the
>>>> device and to retrieve the operating rate of the device.
>>>> This patch also adds the three new APIs in the opp layer
>>>> namely opp_set_rate that can be called to set a new operating
>>>> rate for a device, opp_get_rate that can be called to retrieve
>>>> the operating frequency for a device and opp_populate_rate_fns
>>>> to populte the device specific set_rate and get_rate API's.
>>>> The opp_set_rate and opp_get_rate does some routine error
>>>> checks and finally calls into the device specific set_rate
>>>> and get_rate APIs populated through opp_populate_rate_fns.
>>>>
>>>> Signed-off-by: Thara Gopinath <thara@ti.com>
>>>
>>>As I think about this more, I'm not sure the OPP layer is the right
>>>layer for the get/set rate functions.  The OPP layer is currently just
>>>the data store for OPP data. To me, these set/get functions are better
>>>associated directly with an omap_device instead of an OPP.
>>>
>>>For instance, the new OPP APIs are a bit confusing in terms of OPPs.
>>>e.g: opp_get_rate().  What is the "rate" of an OPP, and what's the
>>>difference with opp_get_freq()?
>>>
>>>What's really happening is the rate is being changed for a device, and
>>>the need for specific hooks are at the device level, not the OPP level.
>>>For example, this current approach would not scale if you needed
>>>multiple devices in the same domain that each needed custom
>>>set_rate/get_rate hooks.
>>>
>>>So instead, what about adding custom hooks at the omap_device level?
>>>They could be registered at omap_device_build() time in the device
>>>specific code.
>
> This is exactly what I had in my mind when I started implementing this.
> But then Paul said hwmod or omap_device is not the place to keep it. 

IIRC, Paul's concern was that *hwmod* was not the right place for this
(and I agree.)  However, I think omap_device is the right place for
this.

Paul?

> Also I do not want the set rate get rate APIs for devices that require
> changes dividers in the PRCM clock module to be spread out in the
> system. Makes it very very difficult to debug.  If we agree to add the
> set_rate and get_rate in the omap_device structure, I would like to
> have one more API in the omap_device layer to register these APIs
> device wise. Thus we can keep these set rate and get rate APIs in one
> single file.

Agreed.  And those functions should be in the respective
device-specific file, where the omap_device_build() etc are done for
that device.

> Also if we move these hooks to omap_device struct we will need to
> rename the current omap_device_set_rate (the main API) to
> omap_device_scale and introduce a new omap_device_set_rate which just
> finds out the omap device from the passed dev pointer and does a
> od->set_rate. Similarly for get rate also.

Yes, that's what I was thinking.

Kevin

RE: [PATCH 01/13] OMAP: Introduce a user list for each voltage domain instance in the voltage driver.

[Gopinath, Thara]

>>-----Original Message-----
>>From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
>>Sent: Thursday, September 16, 2010 8:50 PM
>>To: Gopinath, Thara
>>Cc: linux-omap@vger.kernel.org; paul@pwsan.com; Sripathy, Vishwanath; Sawant, Anand; Cousson, Benoit
>>Subject: Re: [PATCH 01/13] OMAP: Introduce a user list for each voltage domain instance in the
>>voltage driver.
>>
>>"Gopinath, Thara" <thara@ti.com> writes:
>>
>>[...]
>>
>>>>>> + * omap_voltage_add_userreq : API to keep track of various requests to
>>>>>> + *			    scale the VDD and returns the best possible
>>>>>> + *			    voltage the VDD can be put to.
>>>>>> + * @volt_domain: pointer to the voltage domain.
>>>>>> + * @dev : the device pointer.
>>>>>> + * @volt : the voltage which is requested by the device.
>>>>>> + *
>>>>>> + * This API is to be called before the actual voltage scaling is
>>>>>> + * done to determine what is the best possible voltage the VDD can
>>>>>> + * be put to. This API adds the device <dev> in the user list of the
>>>>>> + * vdd <volt_domain> with <volt> as the requested voltage. The user list
>>>>>> + * is a plist with the priority element absolute voltage values.
>>>>>> + * The API then finds the maximum of all the requested voltages for
>>>>>> + * the VDD and returns it back through <volt> pointer itself.
>>>>>> + * Returns error value in case of any errors.
>>>>>> + */
>>>>>> +int omap_voltage_add_userreq(struct voltagedomain *voltdm, struct device *dev,
>>>>>> +		unsigned long *volt)
>>>>>
>>>>>How about just omap_voltage_add_request()
>>>>>
>>>>>Also, do we need both voltdm and dev?  With your other patches, can't we
>>>>>look up the voltm based on dev?  Or, is there a need for a device to add
>>>>>a request in a voltage domain other than the one to which it belongs?
>>>>>
>>>>>Also, how does one remove a voltage request?
>>>
>>> Hello Kevin,
>>>
>>> I could rename this API to what you have suggested. Yes we do need voltdm and dev.
>>> Let us say I have three devices in a voltdm and I need to maintain a request for each
>>> of these devices.
>>
>>OK, thanks for clarifying.
>>
>>> When a omap_device_set_rate API is called by the device to lower its rate the voltage
>>> request will automatically get lowered.
>>
>>OK, but what about removing a request when a device no longer has any
>>voltage constraints.
Hi Kevin,

The devices will use omap_device_set_rate to lower their frequency.
This will in turn call the use counting API in voltage layer and add
which in turn will remove the old request for the device and add the new
request which will be for a lower voltage. Hence releasing gets taken
care of.

Regards
Thara

RE: [PATCH 04/13] OMAP: Introduce API to return a device list associated with a voltage domain

[Gopinath, Thara]

>>-----Original Message-----
>>From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
>>Sent: Thursday, September 16, 2010 8:52 PM
>>To: Gopinath, Thara
>>Cc: linux-omap@vger.kernel.org; paul@pwsan.com; Sripathy, Vishwanath; Sawant, Anand; Cousson, Benoit
>>Subject: Re: [PATCH 04/13] OMAP: Introduce API to return a device list associated with a voltage
>>domain
>>
>>"Gopinath, Thara" <thara@ti.com> writes:
>>
>>[...]
>>
>>>>>> +struct device **opp_init_voltage_params(struct voltagedomain *voltdm,
>>>>>> +					int *dev_count)
>>>>>> +{
>>>>>> +	struct device_opp *dev_opp;
>>>>>> +	struct device **dev_list;
>>>>>> +	int count = 0, i = 0;
>>>>>> +
>>>>>> +	list_for_each_entry(dev_opp, &dev_opp_list, node) {
>>>>>> +		if (!dev_opp->oh->vdd_name)
>>>>>> +			continue;
>>>>>> +
>>>>>> +		if (!strcmp(dev_opp->oh->vdd_name, voltdm->name)) {
>>>>>> +			dev_opp->oh->voltdm = voltdm;
>>>>>
>>>>>Couldn't we assign the voltdm at opp_add() time since you added it as
>>>>>part of the hwmod?
>>>
>>> We cannot as the voltage layer is not initialized at the point of opp_add.
>>> Having said this, today voltage layer is dependent on opp layer only to figure out
>>> the current nominal voltage from the opp table. If that can be some how decoupled we
>>> can initialize voltage layer early on and implement this.
>>
>>We could decouple the voltage init into and early init and late init to
>>handle this.

Hello Kevin,

Yes we could. In fact I do not like the idea of voltage layer dependent
on opp layer itself. So here is my proposal. Maintain a variable in the main
per vdd structure omap_vdd_info curr_volt which will get updated each time a
voltage scale is attempted. Now the only issue is during boot up how does the voltage
layer know what voltage each vdd should be put into ? A piece of code can be
implemented in pm34xx.c/pm44xx.c init functions to read the clock rate associated
with each vdd , call into the opp layer to get the corresponding
voltage ( basically the sequence we do today everytime in voltage layer
when the API to get  nominal voltage is called) and call omap_voltage_scale_vdd with
the corresponding voltage. This way we can fully decouple voltage laye from opp layer
and make voltage init an early_initcall. We might still need a late_initcall to register
debugfs entries though.
What is your take on this ?

Regards
Thara

>>
>>Kevin

RE: [PATCH 05/13] OMAP: Introduce device specific set rate and get rate in device opp structures.

[Gopinath, Thara]

>>-----Original Message-----
>>From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
>>Sent: Thursday, September 16, 2010 8:58 PM
>>To: Gopinath, Thara; Paul Walmsley
>>Cc: linux-omap@vger.kernel.org; Sripathy, Vishwanath; Sawant, Anand; Cousson, Benoit
>>Subject: Re: [PATCH 05/13] OMAP: Introduce device specific set rate and get rate in device opp
>>structures.
>>
>>"Gopinath, Thara" <thara@ti.com> writes:
>>
>>>>>-----Original Message-----
>>>>>From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
>>>>>Sent: Friday, September 03, 2010 5:12 AM
>>>>>To: Gopinath, Thara
>>>>>Cc: linux-omap@vger.kernel.org; paul@pwsan.com; Sripathy, Vishwanath; Sawant, Anand; Cousson,
>>Benoit
>>>>>Subject: Re: [PATCH 05/13] OMAP: Introduce device specific set rate and get rate in device opp
>>>>>structures.
>>>>>
>>>>>Thara Gopinath <thara@ti.com> writes:
>>>>>
>>>>>> This patch extends the device opp structure to contain
>>>>>> pointers to scale the operating rate of the
>>>>>> device and to retrieve the operating rate of the device.
>>>>>> This patch also adds the three new APIs in the opp layer
>>>>>> namely opp_set_rate that can be called to set a new operating
>>>>>> rate for a device, opp_get_rate that can be called to retrieve
>>>>>> the operating frequency for a device and opp_populate_rate_fns
>>>>>> to populte the device specific set_rate and get_rate API's.
>>>>>> The opp_set_rate and opp_get_rate does some routine error
>>>>>> checks and finally calls into the device specific set_rate
>>>>>> and get_rate APIs populated through opp_populate_rate_fns.
>>>>>>
>>>>>> Signed-off-by: Thara Gopinath <thara@ti.com>
>>>>>
>>>>>As I think about this more, I'm not sure the OPP layer is the right
>>>>>layer for the get/set rate functions.  The OPP layer is currently just
>>>>>the data store for OPP data. To me, these set/get functions are better
>>>>>associated directly with an omap_device instead of an OPP.
>>>>>
>>>>>For instance, the new OPP APIs are a bit confusing in terms of OPPs.
>>>>>e.g: opp_get_rate().  What is the "rate" of an OPP, and what's the
>>>>>difference with opp_get_freq()?
>>>>>
>>>>>What's really happening is the rate is being changed for a device, and
>>>>>the need for specific hooks are at the device level, not the OPP level.
>>>>>For example, this current approach would not scale if you needed
>>>>>multiple devices in the same domain that each needed custom
>>>>>set_rate/get_rate hooks.
>>>>>
>>>>>So instead, what about adding custom hooks at the omap_device level?
>>>>>They could be registered at omap_device_build() time in the device
>>>>>specific code.
>>>
>>> This is exactly what I had in my mind when I started implementing this.
>>> But then Paul said hwmod or omap_device is not the place to keep it.
>>
>>IIRC, Paul's concern was that *hwmod* was not the right place for this
>>(and I agree.)  However, I think omap_device is the right place for
>>this.
>>
>>Paul?
>>
>>> Also I do not want the set rate get rate APIs for devices that require
>>> changes dividers in the PRCM clock module to be spread out in the
>>> system. Makes it very very difficult to debug.  If we agree to add the
>>> set_rate and get_rate in the omap_device structure, I would like to
>>> have one more API in the omap_device layer to register these APIs
>>> device wise. Thus we can keep these set rate and get rate APIs in one
>>> single file.
>>
>>Agreed.  And those functions should be in the respective
>>device-specific file, where the omap_device_build() etc are done for
>>that device.

Hello Kevin,

I basically agree with all your other suggestions except this. I do not
think device files are the correct place for this. For starters definitely mpu,
l3 and iva devices are not built from any device file. I do not like the idea
of device set rate APIs spread across different files. 
What I am proposing is like we have  opp_populate_rate_fns(which I have introduced)
we should have something like omap_device_populate_rate_fns. This can be then called
from one file to register the set_rate and get_rate APIs for all the relevant devices
for a Soc. This will be done through an init function. And the device specific
set_rate and get_rate can be implemented in this file. I am not sure which is
this file at this point of time. Maybe for OMAP3 opp3xxx_data.c or pm34xx.c. What
do you think about this?

Regards
Thara

>>
>>> Also if we move these hooks to omap_device struct we will need to
>>> rename the current omap_device_set_rate (the main API) to
>>> omap_device_scale and introduce a new omap_device_set_rate which just
>>> finds out the omap device from the passed dev pointer and does a
>>> od->set_rate. Similarly for get rate also.
>>
>>Yes, that's what I was thinking.
>>
>>Kevin

Re: [PATCH 05/13] OMAP: Introduce device specific set rate and get rate in device opp structures.

[Cousson, Benoit]

Hi Thara,

On 9/17/2010 4:55 PM, Gopinath, Thara wrote:
>
>>> From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
>>> Sent: Thursday, September 16, 2010 8:58 PM
>>>
>>> "Gopinath, Thara"<thara@ti.com>  writes:
>>>
>>>>>> From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
>>>>>> Sent: Friday, September 03, 2010 5:12 AM
>>>>>>
>>>>>> Thara Gopinath<thara@ti.com>  writes:
>>>>>>
>>>>>>> This patch extends the device opp structure to contain
>>>>>>> pointers to scale the operating rate of the
>>>>>>> device and to retrieve the operating rate of the device.
>>>>>>> This patch also adds the three new APIs in the opp layer
>>>>>>> namely opp_set_rate that can be called to set a new operating
>>>>>>> rate for a device, opp_get_rate that can be called to retrieve
>>>>>>> the operating frequency for a device and opp_populate_rate_fns
>>>>>>> to populte the device specific set_rate and get_rate API's.
>>>>>>> The opp_set_rate and opp_get_rate does some routine error
>>>>>>> checks and finally calls into the device specific set_rate
>>>>>>> and get_rate APIs populated through opp_populate_rate_fns.
>>>>>>>
>>>>>>> Signed-off-by: Thara Gopinath<thara@ti.com>
>>>>>>
>>>>>> As I think about this more, I'm not sure the OPP layer is the right
>>>>>> layer for the get/set rate functions.  The OPP layer is currently just
>>>>>> the data store for OPP data. To me, these set/get functions are better
>>>>>> associated directly with an omap_device instead of an OPP.
>>>>>>
>>>>>> For instance, the new OPP APIs are a bit confusing in terms of OPPs.
>>>>>> e.g: opp_get_rate().  What is the "rate" of an OPP, and what's the
>>>>>> difference with opp_get_freq()?
>>>>>>
>>>>>> What's really happening is the rate is being changed for a device, and
>>>>>> the need for specific hooks are at the device level, not the OPP level.
>>>>>> For example, this current approach would not scale if you needed
>>>>>> multiple devices in the same domain that each needed custom
>>>>>> set_rate/get_rate hooks.
>>>>>>
>>>>>> So instead, what about adding custom hooks at the omap_device level?
>>>>>> They could be registered at omap_device_build() time in the device
>>>>>> specific code.
>>>>
>>>> This is exactly what I had in my mind when I started implementing this.
>>>> But then Paul said hwmod or omap_device is not the place to keep it.
>>>
>>> IIRC, Paul's concern was that *hwmod* was not the right place for this
>>> (and I agree.)  However, I think omap_device is the right place for
>>> this.
>>>
>>> Paul?
>>>
>>>> Also I do not want the set rate get rate APIs for devices that require
>>>> changes dividers in the PRCM clock module to be spread out in the
>>>> system. Makes it very very difficult to debug.  If we agree to add the
>>>> set_rate and get_rate in the omap_device structure, I would like to
>>>> have one more API in the omap_device layer to register these APIs
>>>> device wise. Thus we can keep these set rate and get rate APIs in one
>>>> single file.
>>>
>>> Agreed.  And those functions should be in the respective
>>> device-specific file, where the omap_device_build() etc are done for
>>> that device.
>
> Hello Kevin,
>
> I basically agree with all your other suggestions except this. I do not
> think device files are the correct place for this. For starters definitely mpu,
> l3 and iva devices are not built from any device file. I do not like the idea
> of device set rate APIs spread across different files.

I don't not understand the reason?
If we need to add specific device function for mpu, l3, or iva, we can 
easily add a file to contain that.
BTW, thanks to work done by Santosh and Felipe we will probably soon 
introduce a l3 driver that will allow to handle interconnect errors. We 
will thus have a real device for l3 as well.

A device set_rate is clearly device specific. If a device will have to 
play with its own internal dividers along with PRCM dividers, that code 
still belong to the device.

I do not see the need nor the advantage to centralize that? You will 
end-up with a huge file that every driver owners will have to hack in 
order to add set_rate support for their device.

At device build time, IP with set_rate support will just add non-null 
parameters to the omap_device_build(), et voila.

For the debug point of view, you can just add a new sysfs entry for 
scalable devices that will allow you to track scalable device vs regular 
ones.

Regards,
Benoit


> What I am proposing is like we have  opp_populate_rate_fns(which I have introduced)
> we should have something like omap_device_populate_rate_fns. This can be then called
> from one file to register the set_rate and get_rate APIs for all the relevant devices
> for a Soc. This will be done through an init function. And the device specific
> set_rate and get_rate can be implemented in this file. I am not sure which is
> this file at this point of time. Maybe for OMAP3 opp3xxx_data.c or pm34xx.c. What
> do you think about this?
>
> Regards
> Thara
>
>>>
>>>> Also if we move these hooks to omap_device struct we will need to
>>>> rename the current omap_device_set_rate (the main API) to
>>>> omap_device_scale and introduce a new omap_device_set_rate which just
>>>> finds out the omap device from the passed dev pointer and does a
>>>> od->set_rate. Similarly for get rate also.
>>>
>>> Yes, that's what I was thinking.
>>>
>>> Kevin

Re: [PATCH 05/13] OMAP: Introduce device specific set rate and get rate in device opp structures.

[Kevin Hilman]

"Cousson, Benoit" <b-cousson@ti.com> writes:

> Hi Thara,
>
> On 9/17/2010 4:55 PM, Gopinath, Thara wrote:
>>
>>>> From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
>>>> Sent: Thursday, September 16, 2010 8:58 PM
>>>>
>>>> "Gopinath, Thara"<thara@ti.com>  writes:
>>>>
>>>>>>> From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
>>>>>>> Sent: Friday, September 03, 2010 5:12 AM
>>>>>>>
>>>>>>> Thara Gopinath<thara@ti.com>  writes:
>>>>>>>
>>>>>>>> This patch extends the device opp structure to contain
>>>>>>>> pointers to scale the operating rate of the
>>>>>>>> device and to retrieve the operating rate of the device.
>>>>>>>> This patch also adds the three new APIs in the opp layer
>>>>>>>> namely opp_set_rate that can be called to set a new operating
>>>>>>>> rate for a device, opp_get_rate that can be called to retrieve
>>>>>>>> the operating frequency for a device and opp_populate_rate_fns
>>>>>>>> to populte the device specific set_rate and get_rate API's.
>>>>>>>> The opp_set_rate and opp_get_rate does some routine error
>>>>>>>> checks and finally calls into the device specific set_rate
>>>>>>>> and get_rate APIs populated through opp_populate_rate_fns.
>>>>>>>>
>>>>>>>> Signed-off-by: Thara Gopinath<thara@ti.com>
>>>>>>>
>>>>>>> As I think about this more, I'm not sure the OPP layer is the right
>>>>>>> layer for the get/set rate functions.  The OPP layer is currently just
>>>>>>> the data store for OPP data. To me, these set/get functions are better
>>>>>>> associated directly with an omap_device instead of an OPP.
>>>>>>>
>>>>>>> For instance, the new OPP APIs are a bit confusing in terms of OPPs.
>>>>>>> e.g: opp_get_rate().  What is the "rate" of an OPP, and what's the
>>>>>>> difference with opp_get_freq()?
>>>>>>>
>>>>>>> What's really happening is the rate is being changed for a device, and
>>>>>>> the need for specific hooks are at the device level, not the OPP level.
>>>>>>> For example, this current approach would not scale if you needed
>>>>>>> multiple devices in the same domain that each needed custom
>>>>>>> set_rate/get_rate hooks.
>>>>>>>
>>>>>>> So instead, what about adding custom hooks at the omap_device level?
>>>>>>> They could be registered at omap_device_build() time in the device
>>>>>>> specific code.
>>>>>
>>>>> This is exactly what I had in my mind when I started implementing this.
>>>>> But then Paul said hwmod or omap_device is not the place to keep it.
>>>>
>>>> IIRC, Paul's concern was that *hwmod* was not the right place for this
>>>> (and I agree.)  However, I think omap_device is the right place for
>>>> this.
>>>>
>>>> Paul?
>>>>
>>>>> Also I do not want the set rate get rate APIs for devices that require
>>>>> changes dividers in the PRCM clock module to be spread out in the
>>>>> system. Makes it very very difficult to debug.  If we agree to add the
>>>>> set_rate and get_rate in the omap_device structure, I would like to
>>>>> have one more API in the omap_device layer to register these APIs
>>>>> device wise. Thus we can keep these set rate and get rate APIs in one
>>>>> single file.
>>>>
>>>> Agreed.  And those functions should be in the respective
>>>> device-specific file, where the omap_device_build() etc are done for
>>>> that device.
>>
>> Hello Kevin,
>>
>> I basically agree with all your other suggestions except this. I do not
>> think device files are the correct place for this. For starters definitely mpu,
>> l3 and iva devices are not built from any device file. I do not like the idea
>> of device set rate APIs spread across different files.
>
> I don't not understand the reason?
> If we need to add specific device function for mpu, l3, or iva, we can
> easily add a file to contain that.
> BTW, thanks to work done by Santosh and Felipe we will probably soon
> introduce a l3 driver that will allow to handle interconnect
> errors. We will thus have a real device for l3 as well.
>
> A device set_rate is clearly device specific. If a device will have to
> play with its own internal dividers along with PRCM dividers, that
> code still belong to the device.
>
> I do not see the need nor the advantage to centralize that? You will
> end-up with a huge file that every driver owners will have to hack in
> order to add set_rate support for their device.
>
> At device build time, IP with set_rate support will just add non-null
> parameters to the omap_device_build(), et voila.
>
> For the debug point of view, you can just add a new sysfs entry for
> scalable devices that will allow you to track scalable device vs
> regular ones.

I agree with Benoit.

For MPU, IVA/DSP, L3, they are currently done in pm.c because they have
not device init or device file associate with them.   For those, the
new rate functions belong there, since that' where the omap_device_build
is done.

For all others, they should stay in device specific init.

Kevin



>> What I am proposing is like we have  opp_populate_rate_fns(which I have introduced)
>> we should have something like omap_device_populate_rate_fns. This can be then called
>> from one file to register the set_rate and get_rate APIs for all the relevant devices
>> for a Soc. This will be done through an init function. And the device specific
>> set_rate and get_rate can be implemented in this file. I am not sure which is
>> this file at this point of time. Maybe for OMAP3 opp3xxx_data.c or pm34xx.c. What
>> do you think about this?
>>
>> Regards
>> Thara
>>
>>>>
>>>>> Also if we move these hooks to omap_device struct we will need to
>>>>> rename the current omap_device_set_rate (the main API) to
>>>>> omap_device_scale and introduce a new omap_device_set_rate which just
>>>>> finds out the omap device from the passed dev pointer and does a
>>>>> od->set_rate. Similarly for get rate also.
>>>>
>>>> Yes, that's what I was thinking.
>>>>
>>>> Kevin

Re: [PATCH 04/13] OMAP: Introduce API to return a device list associated with a voltage domain

[Kevin Hilman]

"Gopinath, Thara" <thara@ti.com> writes:

>>>-----Original Message-----
>>>From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
>>>Sent: Thursday, September 16, 2010 8:52 PM
>>>To: Gopinath, Thara
>>>Cc: linux-omap@vger.kernel.org; paul@pwsan.com; Sripathy, Vishwanath; Sawant, Anand; Cousson, Benoit
>>>Subject: Re: [PATCH 04/13] OMAP: Introduce API to return a device list associated with a voltage
>>>domain
>>>
>>>"Gopinath, Thara" <thara@ti.com> writes:
>>>
>>>[...]
>>>
>>>>>>> +struct device **opp_init_voltage_params(struct voltagedomain *voltdm,
>>>>>>> +					int *dev_count)
>>>>>>> +{
>>>>>>> +	struct device_opp *dev_opp;
>>>>>>> +	struct device **dev_list;
>>>>>>> +	int count = 0, i = 0;
>>>>>>> +
>>>>>>> +	list_for_each_entry(dev_opp, &dev_opp_list, node) {
>>>>>>> +		if (!dev_opp->oh->vdd_name)
>>>>>>> +			continue;
>>>>>>> +
>>>>>>> +		if (!strcmp(dev_opp->oh->vdd_name, voltdm->name)) {
>>>>>>> +			dev_opp->oh->voltdm = voltdm;
>>>>>>
>>>>>>Couldn't we assign the voltdm at opp_add() time since you added it as
>>>>>>part of the hwmod?
>>>>
>>>> We cannot as the voltage layer is not initialized at the point of opp_add.
>>>> Having said this, today voltage layer is dependent on opp layer only to figure out
>>>> the current nominal voltage from the opp table. If that can be some how decoupled we
>>>> can initialize voltage layer early on and implement this.
>>>
>>>We could decouple the voltage init into and early init and late init to
>>>handle this.
>
> Hello Kevin,
>
> Yes we could. In fact I do not like the idea of voltage layer
> dependent on opp layer itself. So here is my proposal. Maintain a
> variable in the main per vdd structure omap_vdd_info curr_volt which
> will get updated each time a voltage scale is attempted.

OK

> Now the only issue is during boot up how does the voltage layer know
> what voltage each vdd should be put into ? A piece of code can be
> implemented in pm34xx.c/pm44xx.c init functions to read the clock rate
> associated with each vdd , call into the opp layer to get the
> corresponding voltage ( basically the sequence we do today everytime
> in voltage layer when the API to get nominal voltage is called) and
> call omap_voltage_scale_vdd with the corresponding voltage. This way
> we can fully decouple voltage laye from opp layer and make voltage
> init an early_initcall. We might still need a late_initcall to
> register debugfs entries though.  What is your take on this ?

Not sure I fully follow this suggestions, but why not just wait until 
both OPP and voltage late init are fully done before allowing any
voltage scaling?

Alternatively, I just did a quick experiment and the device_initcall in
pm.c (that initializes the MPU, IVA/DSP and L3 devices, as well as does
the OPP layer init) could be made a subsys_initcall, so that by the time
SR + voltage are init'd (using device_initcall), OPP layer is fully
initialized.

Kevin

RE: [PATCH 05/13] OMAP: Introduce device specific set rate and get rate in device opp structures.

[Gopinath, Thara]

>>-----Original Message-----
>>From: Cousson, Benoit
>>Sent: Saturday, September 18, 2010 3:43 PM
>>To: Gopinath, Thara
>>Cc: Kevin Hilman; Paul Walmsley; linux-omap@vger.kernel.org; Sripathy,
>>Vishwanath; Sawant, Anand
>>Subject: Re: [PATCH 05/13] OMAP: Introduce device specific set rate and get
>>rate in device opp structures.
>>
>>Hi Thara,
>>
>>On 9/17/2010 4:55 PM, Gopinath, Thara wrote:
>>>
>>>>> From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
>>>>> Sent: Thursday, September 16, 2010 8:58 PM
>>>>>
>>>>> "Gopinath, Thara"<thara@ti.com>  writes:
>>>>>
>>>>>>>> From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
>>>>>>>> Sent: Friday, September 03, 2010 5:12 AM
>>>>>>>>
>>>>>>>> Thara Gopinath<thara@ti.com>  writes:
>>>>>>>>
>>>>>>>>> This patch extends the device opp structure to contain
>>>>>>>>> pointers to scale the operating rate of the
>>>>>>>>> device and to retrieve the operating rate of the device.
>>>>>>>>> This patch also adds the three new APIs in the opp layer
>>>>>>>>> namely opp_set_rate that can be called to set a new operating
>>>>>>>>> rate for a device, opp_get_rate that can be called to retrieve
>>>>>>>>> the operating frequency for a device and opp_populate_rate_fns
>>>>>>>>> to populte the device specific set_rate and get_rate API's.
>>>>>>>>> The opp_set_rate and opp_get_rate does some routine error
>>>>>>>>> checks and finally calls into the device specific set_rate
>>>>>>>>> and get_rate APIs populated through opp_populate_rate_fns.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Thara Gopinath<thara@ti.com>
>>>>>>>>
>>>>>>>> As I think about this more, I'm not sure the OPP layer is the right
>>>>>>>> layer for the get/set rate functions.  The OPP layer is currently just
>>>>>>>> the data store for OPP data. To me, these set/get functions are better
>>>>>>>> associated directly with an omap_device instead of an OPP.
>>>>>>>>
>>>>>>>> For instance, the new OPP APIs are a bit confusing in terms of OPPs.
>>>>>>>> e.g: opp_get_rate().  What is the "rate" of an OPP, and what's the
>>>>>>>> difference with opp_get_freq()?
>>>>>>>>
>>>>>>>> What's really happening is the rate is being changed for a device, and
>>>>>>>> the need for specific hooks are at the device level, not the OPP
>>level.
>>>>>>>> For example, this current approach would not scale if you needed
>>>>>>>> multiple devices in the same domain that each needed custom
>>>>>>>> set_rate/get_rate hooks.
>>>>>>>>
>>>>>>>> So instead, what about adding custom hooks at the omap_device level?
>>>>>>>> They could be registered at omap_device_build() time in the device
>>>>>>>> specific code.
>>>>>>
>>>>>> This is exactly what I had in my mind when I started implementing this.
>>>>>> But then Paul said hwmod or omap_device is not the place to keep it.
>>>>>
>>>>> IIRC, Paul's concern was that *hwmod* was not the right place for this
>>>>> (and I agree.)  However, I think omap_device is the right place for
>>>>> this.
>>>>>
>>>>> Paul?
>>>>>
>>>>>> Also I do not want the set rate get rate APIs for devices that require
>>>>>> changes dividers in the PRCM clock module to be spread out in the
>>>>>> system. Makes it very very difficult to debug.  If we agree to add the
>>>>>> set_rate and get_rate in the omap_device structure, I would like to
>>>>>> have one more API in the omap_device layer to register these APIs
>>>>>> device wise. Thus we can keep these set rate and get rate APIs in one
>>>>>> single file.
>>>>>
>>>>> Agreed.  And those functions should be in the respective
>>>>> device-specific file, where the omap_device_build() etc are done for
>>>>> that device.
>>>
>>> Hello Kevin,
>>>
>>> I basically agree with all your other suggestions except this. I do not
>>> think device files are the correct place for this. For starters definitely
>>mpu,
>>> l3 and iva devices are not built from any device file. I do not like the
>>idea
>>> of device set rate APIs spread across different files.
>>
>>I don't not understand the reason?
>>If we need to add specific device function for mpu, l3, or iva, we can
>>easily add a file to contain that.
>>BTW, thanks to work done by Santosh and Felipe we will probably soon
>>introduce a l3 driver that will allow to handle interconnect errors. We
>>will thus have a real device for l3 as well.
>>
>>A device set_rate is clearly device specific. If a device will have to
>>play with its own internal dividers along with PRCM dividers, that code
>>still belong to the device.
>>
>>I do not see the need nor the advantage to centralize that? You will
>>end-up with a huge file that every driver owners will have to hack in
>>order to add set_rate support for their device.
>>
>>At device build time, IP with set_rate support will just add non-null
>>parameters to the omap_device_build(), et voila.
>>
>>For the debug point of view, you can just add a new sysfs entry for
>>scalable devices that will allow you to track scalable device vs regular
>>ones.

Hello Benoit/Kevin,

Most of the devices need not scale any internal dividers. For devices that 
need to scale internal dividers, I agree that the set_rate and get_rate should come from the devices file. But for other devices that involve only PRCM dividers, I do not think they should be bothered with implementing these APIs and maintaining them.

Regards
Thara

Re: [PATCH 05/13] OMAP: Introduce device specific set rate and get rate in device opp structures.

[Cousson, Benoit]

Hi Thara,

On 9/29/2010 1:16 PM, Gopinath, Thara wrote:
>
>
>>> -----Original Message-----
>>> From: Cousson, Benoit
>>> Sent: Saturday, September 18, 2010 3:43 PM
>>> To: Gopinath, Thara
>>> Cc: Kevin Hilman; Paul Walmsley; linux-omap@vger.kernel.org; Sripathy,
>>> Vishwanath; Sawant, Anand
>>> Subject: Re: [PATCH 05/13] OMAP: Introduce device specific set rate and get
>>> rate in device opp structures.
>>>
>>> Hi Thara,
>>>
>>> On 9/17/2010 4:55 PM, Gopinath, Thara wrote:
>>>>
>>>>>> From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
>>>>>> Sent: Thursday, September 16, 2010 8:58 PM
>>>>>>
>>>>>> "Gopinath, Thara"<thara@ti.com>   writes:
>>>>>>
>>>>>>>>> From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
>>>>>>>>> Sent: Friday, September 03, 2010 5:12 AM
>>>>>>>>>
>>>>>>>>> Thara Gopinath<thara@ti.com>   writes:
>>>>>>>>>
>>>>>>>>>> This patch extends the device opp structure to contain
>>>>>>>>>> pointers to scale the operating rate of the
>>>>>>>>>> device and to retrieve the operating rate of the device.
>>>>>>>>>> This patch also adds the three new APIs in the opp layer
>>>>>>>>>> namely opp_set_rate that can be called to set a new operating
>>>>>>>>>> rate for a device, opp_get_rate that can be called to retrieve
>>>>>>>>>> the operating frequency for a device and opp_populate_rate_fns
>>>>>>>>>> to populte the device specific set_rate and get_rate API's.
>>>>>>>>>> The opp_set_rate and opp_get_rate does some routine error
>>>>>>>>>> checks and finally calls into the device specific set_rate
>>>>>>>>>> and get_rate APIs populated through opp_populate_rate_fns.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Thara Gopinath<thara@ti.com>
>>>>>>>>>
>>>>>>>>> As I think about this more, I'm not sure the OPP layer is the right
>>>>>>>>> layer for the get/set rate functions.  The OPP layer is currently just
>>>>>>>>> the data store for OPP data. To me, these set/get functions are better
>>>>>>>>> associated directly with an omap_device instead of an OPP.
>>>>>>>>>
>>>>>>>>> For instance, the new OPP APIs are a bit confusing in terms of OPPs.
>>>>>>>>> e.g: opp_get_rate().  What is the "rate" of an OPP, and what's the
>>>>>>>>> difference with opp_get_freq()?
>>>>>>>>>
>>>>>>>>> What's really happening is the rate is being changed for a device, and
>>>>>>>>> the need for specific hooks are at the device level, not the OPP
>>> level.
>>>>>>>>> For example, this current approach would not scale if you needed
>>>>>>>>> multiple devices in the same domain that each needed custom
>>>>>>>>> set_rate/get_rate hooks.
>>>>>>>>>
>>>>>>>>> So instead, what about adding custom hooks at the omap_device level?
>>>>>>>>> They could be registered at omap_device_build() time in the device
>>>>>>>>> specific code.
>>>>>>>
>>>>>>> This is exactly what I had in my mind when I started implementing this.
>>>>>>> But then Paul said hwmod or omap_device is not the place to keep it.
>>>>>>
>>>>>> IIRC, Paul's concern was that *hwmod* was not the right place for this
>>>>>> (and I agree.)  However, I think omap_device is the right place for
>>>>>> this.
>>>>>>
>>>>>> Paul?
>>>>>>
>>>>>>> Also I do not want the set rate get rate APIs for devices that require
>>>>>>> changes dividers in the PRCM clock module to be spread out in the
>>>>>>> system. Makes it very very difficult to debug.  If we agree to add the
>>>>>>> set_rate and get_rate in the omap_device structure, I would like to
>>>>>>> have one more API in the omap_device layer to register these APIs
>>>>>>> device wise. Thus we can keep these set rate and get rate APIs in one
>>>>>>> single file.
>>>>>>
>>>>>> Agreed.  And those functions should be in the respective
>>>>>> device-specific file, where the omap_device_build() etc are done for
>>>>>> that device.
>>>>
>>>> Hello Kevin,
>>>>
>>>> I basically agree with all your other suggestions except this. I do not
>>>> think device files are the correct place for this. For starters definitely
>>> mpu,
>>>> l3 and iva devices are not built from any device file. I do not like the
>>> idea
>>>> of device set rate APIs spread across different files.
>>>
>>> I don't not understand the reason?
>>> If we need to add specific device function for mpu, l3, or iva, we can
>>> easily add a file to contain that.
>>> BTW, thanks to work done by Santosh and Felipe we will probably soon
>>> introduce a l3 driver that will allow to handle interconnect errors. We
>>> will thus have a real device for l3 as well.
>>>
>>> A device set_rate is clearly device specific. If a device will have to
>>> play with its own internal dividers along with PRCM dividers, that code
>>> still belong to the device.
>>>
>>> I do not see the need nor the advantage to centralize that? You will
>>> end-up with a huge file that every driver owners will have to hack in
>>> order to add set_rate support for their device.
>>>
>>> At device build time, IP with set_rate support will just add non-null
>>> parameters to the omap_device_build(), et voila.
>>>
>>> For the debug point of view, you can just add a new sysfs entry for
>>> scalable devices that will allow you to track scalable device vs regular
>>> ones.
>
> Hello Benoit/Kevin,
>
> Most of the devices need not scale any internal dividers. For devices that
> need to scale internal dividers, I agree that the set_rate and get_rate should come from the devices file. But for other devices that involve only PRCM dividers, I do not think they should be bothered with implementing these APIs and maintaining them.

But why? For dividers under PRCM control, we do have clock nodes to 
control them, so device can still use them.

I'm still missing your point about centralizing that.

Benoit

Re: [PATCH 01/13] OMAP: Introduce a user list for each voltage domain instance in the voltage driver.

[Thomas Petazzoni]

Hello Mark,

On Fri, 3 Sep 2010 19:20:52 +0100
Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:

> > re-add? was it there at one point and removed?  Any pointers to the
> > old code?
> 
> It was present in the pre-merge regulator API which you can find in
> the bowels of:
> 
>    git://opensource.wolfsonmicro.com/linux-2.6-audioplus
> 
> but was removed to simplify review during the merge.  It's fairly
> simple to do, it's just that there's been no demand.

I'm finally taking the time to look more closely into this. I've tried
to find the original implementation of multi-consumer tracking, but
couldn't find it in the different branches of the linux-2.6-audioplus.
Do you have a more specific pointer ?

Thanks,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [PATCH 01/13] OMAP: Introduce a user list for each voltage domain instance in the voltage driver.

[Thomas Petazzoni]

Hello Mark,

On Fri, 3 Sep 2010 19:20:52 +0100
Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:

> Essentially all that needs doing is that when regulator_set_voltage() is
> called instead of merging with the machine constraints and applying the
> setting immediately we store the constraints that are specified in the
> consumer then iterate over all enabled consumers applying all the
> constraints that they've set in addition to those from the machine.
> This results in a configuration which is the lowest possible voltage
> which satisfies all the constraints that have been supplied and for
> supplies with only one consumer it gives the same behaviour as we have
> currently.

I went ahead and implemented this (without looking at previous existing
code since I couldn't find it). What about the following patch ?

I've tested it with a dummy platform driver and a dummy regulator
driver, making sure that the correct voltage gets set at the regulator
level. My testing had a device requesting a voltage [830000, 833000]
and another device [822000, 831000]. Without the patch, as the
regulator_set_voltage() gets called for the second device last, the
voltage is set at 822000 which is not acceptable for the first device.
With the patch, the 830000 voltage is kept, since it satisfies both
consumers. If the second device had [822000,828000] has its voltage
requirements, then regulator_set_voltage() would fail with -EINVAL since
it is not possible to find a voltage level that satisfies both
consumers.

If you're ok with it, I'll submit it the proper way.

Thanks,

Thomas


>From fa0edfc1a4428aead4502fcba248084c1194da53 Mon Sep 17 00:00:00 2001
From: Thomas Petazzoni <t-petazzoni@ti.com>
Date: Wed, 24 Nov 2010 10:34:35 +0100
Subject: [PATCH] regulator: Take into account the requirements of all consumers

Extend the regulator_set_voltage() function to take into account the
voltage requirements of all consumers of the regulator being changed,
in order to set the voltage to the minimum voltage acceptable to all
consumers. The existing behaviour was that the latest
regulator_set_voltage() call would win over previous
regulator_set_voltage() calls even if setting the voltage to a
non-acceptable level from other consumers.

Signed-off-by: Thomas Petazzoni <t-petazzoni@ti.com>
---
 drivers/regulator/core.c |   27 +++++++++++++++++++++++++++
 1 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index f1d10c9..12a1cae 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -132,6 +132,28 @@ static int regulator_check_voltage(struct regulator_dev *rdev,
 	return 0;
 }
 
+/* Make sure we select a voltage that suits the needs of all
+ * regulator consumers
+ */
+static int regulator_check_consumers(struct regulator_dev *rdev,
+				     int *min_uV, int *max_uV)
+{
+	struct regulator *regulator;
+
+	list_for_each_entry(regulator, &rdev->consumer_list, list)
+	{
+		if (*max_uV > regulator->max_uV)
+			*max_uV = regulator->max_uV;
+		if (*min_uV < regulator->min_uV)
+			*min_uV = regulator->min_uV;
+	}
+
+	if (*min_uV > *max_uV)
+		return -EINVAL;
+
+	return 0;
+}
+
 /* current constraint check */
 static int regulator_check_current_limit(struct regulator_dev *rdev,
 					int *min_uA, int *max_uA)
@@ -1636,6 +1658,11 @@ int regulator_set_voltage(struct regulator *regulator, int min_uV, int max_uV)
 		goto out;
 	regulator->min_uV = min_uV;
 	regulator->max_uV = max_uV;
+
+	ret = regulator_check_consumers(rdev, &min_uV, &max_uV);
+	if (ret < 0)
+		goto out;
+
 	ret = rdev->desc->ops->set_voltage(rdev, min_uV, max_uV);
 
 out:
-- 
1.7.0.4



-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [PATCH 01/13] OMAP: Introduce a user list for each voltage domain instance in the voltage driver.

[Mark Brown]

On Wed, Nov 24, 2010 at 10:45:44AM +0100, Thomas Petazzoni wrote:

> I went ahead and implemented this (without looking at previous existing
> code since I couldn't find it). What about the following patch ?

That's pretty much exactly what the old code did - told you it was
trivial :)

RE: [PATCH 00/13] OMAP: Basic DVFS Framework

[Vishwanath Sripathy]

Ping.

Vishwa

> -----Original Message-----
> From: Vishwanath BS [mailto:vishwanath.bs@ti.com]
> Sent: Friday, January 21, 2011 7:31 PM
> To: linux-omap@vger.kernel.org
> Cc: patches@linaro.org; Vishwanath BS
> Subject: [PATCH 00/13] OMAP: Basic DVFS Framework
>
> This patch series introduces support for Dynamic Voltage and Frequency
> Scaling
> (DVFS) for OMAP devices.
>
> For detailed design details, refer to DVFS Documentation.
>
> Pending Work:
> 1. OMAP4 support
>
> Changes done in this series:
> 1. Seperated DVFS code from Voltage layer (voltage.c) and introduced
> DVFS layer
>    in dvfs.c
> 2. Added support for frequency throttling and frequency locking (by
> introducing
>    frequency list per device)
> 3. Added changes in omap cpufreq driver for DVFS support
> 4. Fixed race condition issues in DVFS layer
> 5. Added documentation for DVFS framework
> 5. Addressed comments received on V2
> 	V1: https://patchwork.kernel.org/patch/120132/
> 	V2: https://patchwork.kernel.org/patch/290542/
>
> Contributors to conceptualization of the design include
> Anand Sawant <sawant@ti.com>
> Benoit Cousson <b-cousson@ti.com>,
> Kevin Hilman <khilman@deeprootsystems.com>,
> Paul Wamsley <paul@pwsan.com>,
> Parthasarathy Basak <p-basak2@ti.com>
> Thara Gopinath <thara@ti.com>
> Vishwanath Sripathy <vishwanath.bs@ti.com>
>
> This patch series is generated against latest kevin's pm branch and has
> been
> tested on ZOOM3 for mpu, iva and core DVFS.
>
> Thara Gopinath (6):
>   OMAP: Introduce device specific set rate and get rate in omap_device
>     structure
>   OMAP3: Introduce custom set rate and get rate APIs for scalable
>   OMAP: Disable Smartreflex across DVFS
>     devices
>   OMAP3: Introduce voltage domain info in the hwmod structures.
>   OMAP3: Add voltage dependency table for VDD1.
>   OMAP2PLUS: Enable various options in defconfig
>
> Vishwanath BS (7):
>   OMAP: Introduce accessory APIs for DVFS
>   OMAP: Implement Basic DVFS
>   OMAP: Introduce dependent voltage domain support
>   OMAP: Introduce device scale implementation
>   OMAP3: cpufreq driver changes for DVFS support
>   OMAP2PLUS: Replace voltage values with Macros
>   OMAP: Add DVFS Documentation
>
>  Documentation/arm/OMAP/omap_dvfs              |  111 ++++
>  arch/arm/configs/omap2plus_defconfig          |    4 +
>  arch/arm/mach-omap2/Makefile                  |    2 +-
>  arch/arm/mach-omap2/dvfs.c                    |  751
> +++++++++++++++++++++++++
>  arch/arm/mach-omap2/omap_hwmod_3xxx_data.c    |    3 +
>  arch/arm/mach-omap2/opp3xxx_data.c            |   47 +-
>  arch/arm/mach-omap2/opp4xxx_data.c            |   13 +-
>  arch/arm/mach-omap2/pm.c                      |   71 +++
>  arch/arm/mach-omap2/voltage.c                 |  159 ++----
>  arch/arm/plat-omap/cpu-omap.c                 |   35 +-
>  arch/arm/plat-omap/include/plat/dvfs.h        |   34 ++
>  arch/arm/plat-omap/include/plat/omap_device.h |    9 +
>  arch/arm/plat-omap/include/plat/voltage.h     |  148 +++++
>  arch/arm/plat-omap/omap_device.c              |   58 ++
>  14 files changed, 1293 insertions(+), 152 deletions(-)
>  create mode 100644 Documentation/arm/OMAP/omap_dvfs
>  create mode 100644 arch/arm/mach-omap2/dvfs.c
>  create mode 100644 arch/arm/plat-omap/include/plat/dvfs.h

Re: [PATCH 00/13] OMAP: Basic DVFS Framework

[Felipe Balbi]

Hi,

On Mon, Jan 24, 2011 at 12:00:47PM -0800, Kevin Hilman wrote:
> This layer should not be used by drivers, and is OMAP specific.
> 
> The generic interfaces to DVFS for drivers and other kernel code are
> CPUfreq and the regulator framework.

Ok, now it makes sense. Thanks for the info :-)

-- 
balbi

Re: [PATCH 00/13] OMAP: Basic DVFS Framework

[Kevin Hilman]

Felipe Balbi <balbi@ti.com> writes:

> Hi,
>
> On Fri, Jan 21, 2011 at 07:30:52PM +0530, Vishwanath BS wrote:
>> This patch series introduces support for Dynamic Voltage and Frequency Scaling
>> (DVFS) for OMAP devices. 
>> 
>> For detailed design details, refer to DVFS Documentation.
>
> If this is supposed to be used by drivers I would rather not as it's yet
> another OMAP-specific API to use.

This layer should not be used by drivers, and is OMAP specific.

The generic interfaces to DVFS for drivers and other kernel code are
CPUfreq and the regulator framework.

Kevin

Re: [PATCH 00/13] OMAP: Basic DVFS Framework

[Felipe Balbi]

Hi,

On Mon, Jan 24, 2011 at 07:55:21PM +0530, Vishwanath Sripathy wrote:
> It is not just implementation. Even the underlying design of DVFS is
> closely coupled with these layers. If we try to split this DVFS framework
> into generic and OMAP specific part, then the flow will become too
> cumbersome since there are will be too many interactions between common
> and OMAP part. Also it will reduce the code readability aspect as well.
> I do not think it's worth adding some much of complexity and effort just
> to avoid a driver using platform specific function pointers to call these
> APIs.

if what you're saying was really true then we wouldn't have generic
clock API, pm_runtime, pm QoS, CPUFreq, CPUIdle, etc etc. Those are very
much coupled with the underlying Hardware. In fact, every single thing
is very much coupled with the underlying hardware. And even so we manage
to have all of the above plus GPIOLIB, generic IRQ handling (even
threaded), DMA API (which we don't use yet), etc etc etc.

So, IMHO, being coupled to underlying HW is not excuse for not making a
generic API, quite the opposite actually, it should be a motivation.

-- 
balbi

Re: [PATCH 00/13] OMAP: Basic DVFS Framework

[Laurent Pinchart]

On Monday 24 January 2011 15:25:21 Vishwanath Sripathy wrote:
> On Monday, January 24, 2011 11:49 AM Felipe Balbi wrote:
> > On Mon, Jan 24, 2011 at 11:31:20AM +0530, Vishwanath Sripathy wrote:
> > > I do not think DVFS layer can be made a generic layer outside OMAP
> > > because of the fact that DVFS is closely coupled with OMAP device layer
> > > (for getting hwmod related data and clock handling), OMAP voltage layer
> > > (for voltage scaling and handling of dependency voltage domains) and
> > > smart reflex layer.
> > 
> > that an implementation detail. If you:
> > 	a. make the DVFS layer so that you need a HW-glue layer which
> > 	will use OMAP-specific APIs; or
> > 	
> > 	b. pass function pointers for the generic DVFS layer to use
> > 
> > (note that I'd rather have option (a)), you solve the problem, no ?
> 
> It is not just implementation. Even the underlying design of DVFS is
> closely coupled with these layers. If we try to split this DVFS framework
> into generic and OMAP specific part, then the flow will become too
> cumbersome since there are will be too many interactions between common
> and OMAP part. Also it will reduce the code readability aspect as well.
> I do not think it's worth adding some much of complexity and effort just
> to avoid a driver using platform specific function pointers to call these
> APIs.

The issue with callbacks to board code is that they prevent the use of the 
device tree which is getting introduced on ARM platforms. We should avoid them 
as much as possible.

-- 
Regards,

Laurent Pinchart

RE: [PATCH 00/13] OMAP: Basic DVFS Framework

[Vishwanath Sripathy]

Balbi,

> -----Original Message-----
> From: Felipe Balbi [mailto:balbi@ti.com]
> Sent: Monday, January 24, 2011 11:49 AM
> To: Vishwanath Sripathy
> Cc: balbi@ti.com; linux-omap@vger.kernel.org; patches@linaro.org
> Subject: Re: [PATCH 00/13] OMAP: Basic DVFS Framework
>
> Hi,
>
> On Mon, Jan 24, 2011 at 11:31:20AM +0530, Vishwanath Sripathy
> wrote:
> > I do not think DVFS layer can be made a generic layer outside OMAP
> because
> > of the fact that DVFS is closely coupled with OMAP device layer (for
> > getting hwmod related data and clock handling), OMAP voltage layer
> (for
> > voltage scaling and handling of dependency voltage domains) and
> smart
> > reflex layer.
>
> that an implementation detail. If you:
>
> 	a. make the DVFS layer so that you need a HW-glue layer which
> 	will use OMAP-specific APIs; or
>
> 	b. pass function pointers for the generic DVFS layer to use
>
> (note that I'd rather have option (a)), you solve the problem, no ?
It is not just implementation. Even the underlying design of DVFS is
closely coupled with these layers. If we try to split this DVFS framework
into generic and OMAP specific part, then the flow will become too
cumbersome since there are will be too many interactions between common
and OMAP part. Also it will reduce the code readability aspect as well.
I do not think it's worth adding some much of complexity and effort just
to avoid a driver using platform specific function pointers to call these
APIs.

Vishwa

>
> --
> balbi

Re: [PATCH 00/13] OMAP: Basic DVFS Framework

[Felipe Balbi]

Hi,

On Mon, Jan 24, 2011 at 11:31:20AM +0530, Vishwanath Sripathy wrote:
> I do not think DVFS layer can be made a generic layer outside OMAP because
> of the fact that DVFS is closely coupled with OMAP device layer (for
> getting hwmod related data and clock handling), OMAP voltage layer (for
> voltage scaling and handling of dependency voltage domains) and smart
> reflex layer.

that an implementation detail. If you:

	a. make the DVFS layer so that you need a HW-glue layer which
	will use OMAP-specific APIs; or

	b. pass function pointers for the generic DVFS layer to use

(note that I'd rather have option (a)), you solve the problem, no ?

-- 
balbi

RE: [PATCH 00/13] OMAP: Basic DVFS Framework

[Vishwanath Sripathy]

Balbi,

> -----Original Message-----
> From: Felipe Balbi [mailto:balbi@ti.com]
> Sent: Saturday, January 22, 2011 10:48 PM
> To: Vishwanath BS
> Cc: linux-omap@vger.kernel.org; patches@linaro.org
> Subject: Re: [PATCH 00/13] OMAP: Basic DVFS Framework
>
> Hi,
>
> On Fri, Jan 21, 2011 at 07:30:52PM +0530, Vishwanath BS wrote:
> > This patch series introduces support for Dynamic Voltage and
> Frequency Scaling
> > (DVFS) for OMAP devices.
> >
> > For detailed design details, refer to DVFS Documentation.
>
> If this is supposed to be used by drivers I would rather not as it's yet
> another OMAP-specific API to use.
>
> Before you reply with "you can pass function pointers via platform_data"
> let me say that even that is a burden, specially on devices which are
> used by several archs.
>
> Why don't you make it more generic like the OPP layer was created ?
> Isn't it better to just build on top of the OPP layer and make this one
> also generic ?
I do not think DVFS layer can be made a generic layer outside OMAP because
of the fact that DVFS is closely coupled with OMAP device layer (for
getting hwmod related data and clock handling), OMAP voltage layer (for
voltage scaling and handling of dependency voltage domains) and smart
reflex layer.

Vishwa

>
> --
> balbi

Re: [PATCH 00/13] OMAP: Basic DVFS Framework

[Felipe Balbi]

Hi,

On Fri, Jan 21, 2011 at 07:30:52PM +0530, Vishwanath BS wrote:
> This patch series introduces support for Dynamic Voltage and Frequency Scaling
> (DVFS) for OMAP devices. 
> 
> For detailed design details, refer to DVFS Documentation.

If this is supposed to be used by drivers I would rather not as it's yet
another OMAP-specific API to use.

Before you reply with "you can pass function pointers via platform_data"
let me say that even that is a burden, specially on devices which are
used by several archs.

Why don't you make it more generic like the OPP layer was created ?
Isn't it better to just build on top of the OPP layer and make this one
also generic ?

-- 
balbi

[PATCH 00/13] OMAP: Basic DVFS Framework

[Vishwanath BS]

This patch series introduces support for Dynamic Voltage and Frequency Scaling
(DVFS) for OMAP devices. 

For detailed design details, refer to DVFS Documentation.

Pending Work:
1. OMAP4 support

Changes done in this series:
1. Seperated DVFS code from Voltage layer (voltage.c) and introduced DVFS layer
   in dvfs.c
2. Added support for frequency throttling and frequency locking (by introducing
   frequency list per device)
3. Added changes in omap cpufreq driver for DVFS support
4. Fixed race condition issues in DVFS layer
5. Added documentation for DVFS framework
5. Addressed comments received on V2
	V1: https://patchwork.kernel.org/patch/120132/
	V2: https://patchwork.kernel.org/patch/290542/

Contributors to conceptualization of the design include
Anand Sawant <sawant@ti.com>
Benoit Cousson <b-cousson@ti.com>,
Kevin Hilman <khilman@deeprootsystems.com>,
Paul Wamsley <paul@pwsan.com>,
Parthasarathy Basak <p-basak2@ti.com>
Thara Gopinath <thara@ti.com>
Vishwanath Sripathy <vishwanath.bs@ti.com>

This patch series is generated against latest kevin's pm branch and has been
tested on ZOOM3 for mpu, iva and core DVFS.

Thara Gopinath (6):
  OMAP: Introduce device specific set rate and get rate in omap_device
    structure
  OMAP3: Introduce custom set rate and get rate APIs for scalable
  OMAP: Disable Smartreflex across DVFS
    devices
  OMAP3: Introduce voltage domain info in the hwmod structures.
  OMAP3: Add voltage dependency table for VDD1.
  OMAP2PLUS: Enable various options in defconfig

Vishwanath BS (7):
  OMAP: Introduce accessory APIs for DVFS
  OMAP: Implement Basic DVFS
  OMAP: Introduce dependent voltage domain support
  OMAP: Introduce device scale implementation
  OMAP3: cpufreq driver changes for DVFS support
  OMAP2PLUS: Replace voltage values with Macros
  OMAP: Add DVFS Documentation

 Documentation/arm/OMAP/omap_dvfs              |  111 ++++
 arch/arm/configs/omap2plus_defconfig          |    4 +
 arch/arm/mach-omap2/Makefile                  |    2 +-
 arch/arm/mach-omap2/dvfs.c                    |  751 +++++++++++++++++++++++++
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c    |    3 +
 arch/arm/mach-omap2/opp3xxx_data.c            |   47 +-
 arch/arm/mach-omap2/opp4xxx_data.c            |   13 +-
 arch/arm/mach-omap2/pm.c                      |   71 +++
 arch/arm/mach-omap2/voltage.c                 |  159 ++----
 arch/arm/plat-omap/cpu-omap.c                 |   35 +-
 arch/arm/plat-omap/include/plat/dvfs.h        |   34 ++
 arch/arm/plat-omap/include/plat/omap_device.h |    9 +
 arch/arm/plat-omap/include/plat/voltage.h     |  148 +++++
 arch/arm/plat-omap/omap_device.c              |   58 ++
 14 files changed, 1293 insertions(+), 152 deletions(-)
 create mode 100644 Documentation/arm/OMAP/omap_dvfs
 create mode 100644 arch/arm/mach-omap2/dvfs.c
 create mode 100644 arch/arm/plat-omap/include/plat/dvfs.h