[PATCH 00/13] OMAP: Basic DVFS Framework

[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

[PATCH 01/13] OMAP: Introduce accessory APIs for DVFS

[Vishwanath BS]

This patch introduces accessory APIs for DVFS.
Data structures added:
1. omap_dev_user_list: This structure maintains list of frequency requests per
   device basis. When a device needs to be scaled to a particular frequency,
   This list is searched to find the maximum request for a given device.
   If noone has placed any request, device frequency is obtained from device
   opp table.
2. omap_vdd_dev_list: This strcucture stores device list per vdd basis.
   Whenever a device is registered with a vdd, it is added to this list.
3. omap_vdd_user_list: 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.
4. omap_vdd_dvfs_info: This structure is used to abstract DVFS related
   information per VDD basis. It holds pointer to corresponding vdd's
   voltagedomain instance and pointer to user list.

Following APIs have been added to operate on above data structures:
1. omap_dvfs_add_vdd_user - API to add a user into omap_vdd_user_list
2. omap_vdd_user_list - API to remove a user from omap_vdd_user_list
3. omap_dvfs_register_device - API to register a device with vdd
4. omap_dvfs_add_freq_request - API to add a frequency request into
   omap_dev_user_list
5. omap_dvfs_remove_freq_request - API to remove a frequency request from
   omap_dev_user_list
6. omap_dvfs_find_voltage - API to find the opp corresponding to given voltage

DVFS layer is initialized and basic data structures are allocated and
initialized as part of this.

This patch is based on Thara's previous DVFS implementation, but with major
rework.

Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
Cc: Thara Gopinath <thara@ti.com>
---
 arch/arm/mach-omap2/Makefile           |    2 +-
 arch/arm/mach-omap2/dvfs.c             |  456 ++++++++++++++++++++++++++++++++
 arch/arm/plat-omap/include/plat/dvfs.h |   27 ++
 arch/arm/plat-omap/omap_device.c       |    9 +
 4 files changed, 493 insertions(+), 1 deletions(-)
 create mode 100644 arch/arm/mach-omap2/dvfs.c
 create mode 100644 arch/arm/plat-omap/include/plat/dvfs.h

diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
index 4ab82f6..bb2e2bc 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -61,7 +61,7 @@ ifeq ($(CONFIG_PM),y)
 obj-$(CONFIG_ARCH_OMAP2)		+= pm24xx.o
 obj-$(CONFIG_ARCH_OMAP2)		+= sleep24xx.o pm_bus.o voltage.o
 obj-$(CONFIG_ARCH_OMAP3)		+= pm34xx.o sleep34xx.o voltage.o \
-					   cpuidle34xx.o pm_bus.o
+					   cpuidle34xx.o pm_bus.o dvfs.o
 obj-$(CONFIG_ARCH_OMAP4)		+= pm44xx.o voltage.o pm_bus.o
 obj-$(CONFIG_PM_DEBUG)			+= pm-debug.o
 obj-$(CONFIG_OMAP_SMARTREFLEX)          += sr_device.o smartreflex.o
diff --git a/arch/arm/mach-omap2/dvfs.c b/arch/arm/mach-omap2/dvfs.c
new file mode 100644
index 0000000..8832e4a
--- /dev/null
+++ b/arch/arm/mach-omap2/dvfs.c
@@ -0,0 +1,456 @@
+/*
+ * OMAP3/OMAP4 DVFS Management Routines
+ *
+ * Author: Vishwanath BS	<vishwanath.bs@ti.com>
+ *
+ * Copyright (C) 2011 Texas Instruments, Inc.
+ * Vishwanath BS <vishwanath.bs@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/err.h>
+#include <linux/spinlock.h>
+#include <linux/plist.h>
+#include <linux/slab.h>
+#include <linux/opp.h>
+#include <plat/common.h>
+#include <plat/voltage.h>
+#include <plat/omap_device.h>
+
+/**
+ * struct omap_dev_user_list - Structure maitain userlist per devide
+ *
+ * @dev:       The device requesting for a particular frequency
+ * @node:      The list head entry
+ * @freq:      frequency being requested
+ *
+ * Using this structure, user list (requesting dev * and frequency) for
+ * each device is maintained. This is how we can have different devices
+ * at different frequencies (to support frequency locking and throttling).
+ * Even if one of the devices in a given vdd has locked it's frequency,
+ * other's can still scale their frequency using this list.
+ * If no one has placed a frequency request for a device, then device is
+ * set to the frequency from it's opp table.
+ */
+struct omap_dev_user_list {
+		struct device *dev;
+		struct plist_node node;
+		u32 freq;
+};
+
+/**
+ * struct omap_vdd_dev_list - Device list per vdd
+ *
+ * @dev:	The device belonging to a particular vdd
+ * @node:	The list head entry
+ */
+struct omap_vdd_dev_list {
+	struct device *dev;
+	struct list_head node;
+	struct plist_head user_list;
+	spinlock_t user_lock; /* spinlock for plist */
+};
+
+/**
+ * struct 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;
+};
+
+/**
+ * struct omap_vdd_dvfs_info - The per vdd dvfs info
+ *
+ * @user_lock:	spinlock for plist operations
+ * @user_list:	The vdd user list
+ * @scaling_mutex:	Mutex for protecting dvfs data structures for a vdd
+ * @voltdm: Voltage domains for which dvfs info stored
+ *
+ * This is a fundamental structure used to store all the required
+ * DVFS related information for a vdd.
+ */
+struct omap_vdd_dvfs_info {
+	spinlock_t user_lock; /* spin lock */
+	struct plist_head user_list;
+	struct mutex scaling_mutex; /* dvfs mutex */
+	struct voltagedomain *voltdm;
+	struct list_head dev_list;
+};
+
+static struct omap_vdd_dvfs_info *omap_dvfs_info_list;
+static int omap_nr_vdd;
+
+static struct voltagedomain omap3_vdd[] = {
+	{
+	.name = "mpu",
+	},
+	{
+	.name = "core",
+	},
+};
+
+static int __init omap_dvfs_init(void);
+
+static struct omap_vdd_dvfs_info *get_dvfs_info(struct voltagedomain *voltdm)
+{
+	int i;
+	if (!voltdm || !omap_dvfs_info_list)
+		return NULL;
+
+	for (i = 0; i < omap_nr_vdd; i++)
+		if (omap_dvfs_info_list[i].voltdm == voltdm)
+			return &omap_dvfs_info_list[i];
+
+	pr_warning("%s: unable find dvfs info for vdd %s\n",
+			__func__, voltdm->name);
+	return NULL;
+}
+
+/**
+ * omap_dvfs_find_voltage() - search for given 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. If there are multiple opps with same voltage, it will return
+ * the first available entry.
+ */
+static struct opp *omap_dvfs_find_voltage(struct device *dev,
+		unsigned long volt)
+{
+	struct opp *opp = ERR_PTR(-ENODEV);
+	unsigned long f = 0;
+
+	do {
+		opp = opp_find_freq_ceil(dev, &f);
+		if (IS_ERR(opp))
+			break;
+		if (opp_get_voltage(opp) >= volt)
+			break;
+		f++;
+	} while (1);
+
+	return opp;
+}
+
+/**
+ * omap_dvfs_add_vdd_user() - Add a voltage request
+ * @dvfs_info: omap_vdd_dvfs_info pointer for the required vdd
+ * @dev: device making the request
+ * @volt: requesting voltage in uV
+ *
+ * Adds the given devices' voltage request into corresponding
+ * vdd's omap_vdd_dvfs_info user list (plist). This list is used
+ * to find the maximum voltage request for a given vdd.
+ *
+ * Returns 0 on success.
+ */
+static int omap_dvfs_add_vdd_user(struct omap_vdd_dvfs_info *dvfs_info,
+		struct device *dev, unsigned long volt)
+{
+	struct omap_vdd_user_list *user = NULL, *temp_user;
+	struct plist_node *node;
+
+	if (!dvfs_info || IS_ERR(dvfs_info)) {
+		dev_warn(dev, "%s: VDD specified does not exist!\n", __func__);
+		return -EINVAL;
+	}
+
+	mutex_lock(&dvfs_info->scaling_mutex);
+
+	plist_for_each_entry(temp_user, &dvfs_info->user_list, node) {
+		if (temp_user->dev == dev) {
+			user = temp_user;
+			break;
+		}
+	}
+
+	if (!user) {
+		user = kzalloc(sizeof(struct omap_vdd_user_list), GFP_KERNEL);
+		if (!user) {
+			dev_err(dev, "%s: Unable to creat a new user for vdd_%s\n",
+				__func__, dvfs_info->voltdm->name);
+			mutex_unlock(&dvfs_info->scaling_mutex);
+			return -ENOMEM;
+		}
+		user->dev = dev;
+	} else {
+		plist_del(&user->node, &dvfs_info->user_list);
+	}
+
+	plist_node_init(&user->node, volt);
+	plist_add(&user->node, &dvfs_info->user_list);
+	node = plist_last(&dvfs_info->user_list);
+	user->volt = node->prio;
+
+	mutex_unlock(&dvfs_info->scaling_mutex);
+
+	return 0;
+}
+
+/**
+ * omap_dvfs_remove_vdd_user() - Remove a voltage request
+ * @dvfs_info: omap_vdd_dvfs_info pointer for the required vdd
+ * @dev: device making the request
+ *
+ * Removes the given devices' voltage request from corresponding
+ * vdd's omap_vdd_dvfs_info user list (plist).
+ *
+ * Returns 0 on success.
+ */
+static int omap_dvfs_remove_vdd_user(struct omap_vdd_dvfs_info *dvfs_info,
+		struct device *dev)
+{
+	struct omap_vdd_user_list *user = NULL, *temp_user;
+	int ret = 0;
+
+	if (!dvfs_info || IS_ERR(dvfs_info)) {
+		dev_err(dev, "%s: VDD specified does not exist!\n", __func__);
+		return -EINVAL;
+	}
+
+	mutex_lock(&dvfs_info->scaling_mutex);
+
+	plist_for_each_entry(temp_user, &dvfs_info->user_list, node) {
+		if (temp_user->dev == dev) {
+			user = temp_user;
+			break;
+		}
+	}
+
+	if (user)
+		plist_del(&user->node, &dvfs_info->user_list);
+	else {
+		dev_err(dev, "%s: Unable to find the user for vdd_%s\n",
+					__func__, dvfs_info->voltdm->name);
+		ret = -ENOMEM;
+	}
+	mutex_unlock(&dvfs_info->scaling_mutex);
+
+	return ret;
+}
+
+/**
+ * omap_dvfs_register_device - Add a device into voltage domain
+ * @voltdm:	voltage domain to which the device is to be added
+ * @dev:	Device to be added
+ *
+ * This API will add a given device into user_list of corresponding
+ * vdd's omap_vdd_dvfs_info strucure. This list is traversed to scale
+ * frequencies of all the devices on a given vdd. This api is called
+ * while hwmod db is built for an omap_device.
+ *
+ * Returns 0 on success.
+ */
+int omap_dvfs_register_device(struct voltagedomain *voltdm, struct device *dev)
+{
+	struct omap_vdd_dev_list *temp_dev;
+	struct omap_vdd_dvfs_info *dvfs_info = get_dvfs_info(voltdm);
+
+	if (!voltdm || IS_ERR(voltdm) || !dvfs_info) {
+		dev_warn(dev, "%s: VDD specified does not exist!\n", __func__);
+		return -EINVAL;
+	}
+
+	list_for_each_entry(temp_dev, &dvfs_info->dev_list, node) {
+		if (temp_dev->dev == dev) {
+			dev_warn(dev, "%s: Device already added to vdee_%s\n",
+				__func__, dvfs_info->voltdm->name);
+			return -EINVAL;
+		}
+	}
+
+	temp_dev = kzalloc(sizeof(struct omap_vdd_dev_list), GFP_KERNEL);
+	if (!temp_dev) {
+		dev_err(dev, "%s: Unable to creat a new device for vdd_%s\n",
+			__func__, dvfs_info->voltdm->name);
+		return -ENOMEM;
+	}
+
+	/* Initialize priority ordered list */
+	spin_lock_init(&temp_dev->user_lock);
+	plist_head_init(&temp_dev->user_list, &temp_dev->user_lock);
+
+	temp_dev->dev = dev;
+	list_add(&temp_dev->node, &dvfs_info->dev_list);
+
+	return 0;
+}
+
+/**
+ * omap_dvfs_add_freq_request() - add a requested device frequency
+ *
+ *
+ * @dvfs_info: omap_vdd_dvfs_info pointer for the required vdd
+ * @req_dev: device making the request
+ * @target_dev: target device for which frequency request is being made
+ * @freq:	target device frequency
+ *
+ * This API adds a requested frequency into target's device frequency list.
+ *
+ * Returns 0 on success.
+ */
+static int omap_dvfs_add_freq_request(struct omap_vdd_dvfs_info *dvfs_info,
+	struct device *req_dev, struct device *target_dev, unsigned long freq)
+{
+	struct omap_dev_user_list *dev_user = NULL, *tmp_user;
+	struct omap_vdd_dev_list *temp_dev;
+
+	if (!dvfs_info || IS_ERR(dvfs_info)) {
+		dev_warn(target_dev, "%s: VDD specified does not exist!\n",
+			__func__);
+		return -EINVAL;
+	}
+
+	mutex_lock(&dvfs_info->scaling_mutex);
+
+	list_for_each_entry(temp_dev, &dvfs_info->dev_list, node) {
+		if (temp_dev->dev == target_dev)
+			break;
+	}
+
+	if (temp_dev->dev != target_dev) {
+		dev_warn(target_dev, "%s: target_dev does not exist!\n",
+			__func__);
+		mutex_unlock(&dvfs_info->scaling_mutex);
+		return -EINVAL;
+	}
+
+	plist_for_each_entry(tmp_user, &temp_dev->user_list, node) {
+		if (tmp_user->dev == req_dev) {
+			dev_user = tmp_user;
+			break;
+		}
+	}
+
+	if (!dev_user) {
+		dev_user = kzalloc(sizeof(struct omap_dev_user_list),
+					GFP_KERNEL);
+		if (!dev_user) {
+			dev_err(target_dev, "%s: Unable to creat a new user for vdd_%s\n",
+				__func__, dvfs_info->voltdm->name);
+			mutex_unlock(&dvfs_info->scaling_mutex);
+			return -ENOMEM;
+		}
+		dev_user->dev = req_dev;
+	} else {
+		plist_del(&dev_user->node, &temp_dev->user_list);
+	}
+
+	plist_node_init(&dev_user->node, freq);
+	plist_add(&dev_user->node, &temp_dev->user_list);
+
+	mutex_unlock(&dvfs_info->scaling_mutex);
+	return 0;
+}
+
+/**
+ * omap_dvfs_remove_freq_request() - Remove the requested device frequency
+ *
+ * @dvfs_info: omap_vdd_dvfs_info pointer for the required vdd
+ * @req_dev: device removing the request
+ * @target_dev: target device from which frequency request is being removed
+ *
+ * This API removes a requested frequency from target's device frequency list.
+ *
+ * Returns 0 on success.
+ */
+
+static int omap_dvfs_remove_freq_request(struct omap_vdd_dvfs_info *dvfs_info,
+	struct device *req_dev, struct device *target_dev)
+{
+	struct omap_dev_user_list *dev_user = NULL, *tmp_user;
+	int ret = 0;
+	struct omap_vdd_dev_list *temp_dev;
+
+	if (!dvfs_info || IS_ERR(dvfs_info)) {
+		dev_warn(target_dev, "%s: VDD specified does not exist!\n",
+			__func__);
+		return -EINVAL;
+	}
+
+	mutex_lock(&dvfs_info->scaling_mutex);
+
+	list_for_each_entry(temp_dev, &dvfs_info->dev_list, node) {
+		if (temp_dev->dev == target_dev)
+			break;
+	}
+
+	if (temp_dev->dev != target_dev) {
+		dev_warn(target_dev, "%s: target_dev does not exist!\n",
+			__func__);
+		mutex_unlock(&dvfs_info->scaling_mutex);
+		return -EINVAL;
+	}
+
+	plist_for_each_entry(tmp_user, &temp_dev->user_list, node) {
+		if (tmp_user->dev == req_dev) {
+			dev_user = tmp_user;
+			break;
+		}
+	}
+
+	if (dev_user)
+		plist_del(&dev_user->node, &temp_dev->user_list);
+	else {
+		dev_err(target_dev, "%s: Unable to remove the user for vdd_%s\n",
+				__func__, dvfs_info->voltdm->name);
+			ret = -EINVAL;
+		}
+
+	return ret;
+}
+
+/**
+ * omap_dvfs_init() - Initialize omap dvfs layer
+ *
+ * Initalizes omap dvfs layer. It basically allocates memory for
+ * omap_dvfs_info_list and  populates voltdm pointer inside
+ * omap_vdd_dvfs_info structure for all the VDDs.
+ *
+ * Returns 0 on success.
+ */
+static int __init omap_dvfs_init()
+{
+	int i;
+	struct voltagedomain *vdd_list;
+	if (cpu_is_omap34xx()) {
+		omap_nr_vdd = 2;
+		vdd_list = omap3_vdd;
+	}
+
+	omap_dvfs_info_list = kzalloc(omap_nr_vdd *
+			sizeof(struct omap_vdd_dvfs_info), GFP_KERNEL);
+	if (!omap_dvfs_info_list) {
+		pr_warning("%s: Unable to allocate memory for vdd_list",
+			__func__);
+		return -ENOMEM;
+	}
+
+	for (i = 0; i < omap_nr_vdd; i++) {
+		omap_dvfs_info_list[i].voltdm =
+			omap_voltage_domain_lookup(vdd_list[i].name);
+		/* Init the plist */
+		spin_lock_init(&omap_dvfs_info_list[i].user_lock);
+		plist_head_init(&omap_dvfs_info_list[i].user_list,
+					&omap_dvfs_info_list[i].user_lock);
+		/* Init the DVFS mutex */
+		mutex_init(&omap_dvfs_info_list[i].scaling_mutex);
+		/* Init the device list */
+		INIT_LIST_HEAD(&omap_dvfs_info_list[i].dev_list);
+	}
+
+	return 0;
+}
+core_initcall(omap_dvfs_init);
diff --git a/arch/arm/plat-omap/include/plat/dvfs.h b/arch/arm/plat-omap/include/plat/dvfs.h
new file mode 100644
index 0000000..1302990
--- /dev/null
+++ b/arch/arm/plat-omap/include/plat/dvfs.h
@@ -0,0 +1,27 @@
+/*
+ * OMAP3/OMAP4 DVFS Management Routines
+ *
+ * Author: Vishwanath BS	<vishwanath.bs@ti.com>
+ *
+ * Copyright (C) 2011 Texas Instruments, Inc.
+ * Vishwanath BS <vishwanath.bs@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __ARCH_ARM_MACH_OMAP2_DVFS_H
+#define __ARCH_ARM_MACH_OMAP2_DVFS_H
+#include <plat/voltage.h>
+
+#ifdef CONFIG_PM
+int omap_dvfs_register_device(struct voltagedomain *voltdm, struct device *dev);
+#else
+static inline int omap_dvfs_register_device(struct voltagedomain *voltdm,
+		struct device *dev)
+{
+	return -EINVAL;
+}
+#endif
+#endif
diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
index 57adb27..a84e849 100644
--- a/arch/arm/plat-omap/omap_device.c
+++ b/arch/arm/plat-omap/omap_device.c
@@ -86,6 +86,7 @@
 
 #include <plat/omap_device.h>
 #include <plat/omap_hwmod.h>
+#include <plat/dvfs.h>
 
 /* These parameters are passed to _omap_device_{de,}activate() */
 #define USE_WAKEUP_LAT			0
@@ -481,6 +482,14 @@ struct omap_device *omap_device_build_ss(const char *pdev_name, int pdev_id,
 	for (i = 0; i < oh_cnt; i++) {
 		hwmods[i]->od = od;
 		_add_optional_clock_alias(od, hwmods[i]);
+		if (!is_early_device && hwmods[i]->vdd_name) {
+			struct omap_hwmod *oh = hwmods[i];
+			struct voltagedomain *voltdm;
+
+			voltdm = omap_voltage_domain_lookup(oh->vdd_name);
+			if (!omap_dvfs_register_device(voltdm, &od->pdev.dev))
+				oh->voltdm = voltdm;
+		}
 	}
 
 	if (ret)
-- 
1.7.0.4

[PATCH 02/13] OMAP: Introduce device specific set rate and get rate in omap_device structure

[Vishwanath BS]

From: Thara Gopinath <thara@ti.com>

This patch extends the omap_device 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 omap device layer
namely omap_device_set_rate that can be called to set a new operating
rate for a device, omap_device_get_rate that can be called to retrieve
the operating frequency for a device and omap_device_register_dvfs_callbacks
to register the device specific set_rate and get_rate functions.
The omap_device_set_rate and omap_device_get_rate does some routine error
checks and finally calls into the device specific set_rate
and get_rate APIs populated through omap_device_populate_rate_fns.

Signed-off-by: Thara Gopinath <thara@ti.com>
Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
---
 arch/arm/plat-omap/include/plat/omap_device.h |    9 +++++
 arch/arm/plat-omap/omap_device.c              |   49 +++++++++++++++++++++++++
 2 files changed, 58 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 e4c349f..204fb0a 100644
--- a/arch/arm/plat-omap/include/plat/omap_device.h
+++ b/arch/arm/plat-omap/include/plat/omap_device.h
@@ -50,6 +50,8 @@ extern struct device omap_device_parent;
  * @hwmods: (one .. many per omap_device)
  * @hwmods_cnt: ARRAY_SIZE() of @hwmods
  * @pm_lats: ptr to an omap_device_pm_latency table
+ * @set_rate: fn ptr to change the operating rate.
+ * @get_rate: fn ptr to retrieve the current operating rate.
  * @pm_lats_cnt: ARRAY_SIZE() of what is passed to @pm_lats
  * @pm_lat_level: array index of the last odpl entry executed - -1 if never
  * @dev_wakeup_lat: dev wakeup latency in nanoseconds
@@ -73,6 +75,8 @@ struct omap_device {
 	s8				pm_lat_level;
 	u8				hwmods_cnt;
 	u8				_state;
+	int (*set_rate)(struct device *dev, unsigned long rate);
+	unsigned long (*get_rate) (struct device *dev);
 };
 
 /* Device driver interface (call via platform_data fn ptrs) */
@@ -107,6 +111,11 @@ void __iomem *omap_device_get_rt_va(struct omap_device *od);
 int omap_device_align_pm_lat(struct platform_device *pdev,
 			     u32 new_wakeup_lat_limit);
 struct powerdomain *omap_device_get_pwrdm(struct omap_device *od);
+int omap_device_set_rate(struct device *dev, unsigned long freq);
+unsigned long omap_device_get_rate(struct device *dev);
+void omap_device_register_dvfs_callbacks(struct device *dev,
+		int (*set_rate)(struct device *dev, unsigned long rate),
+		unsigned long (*get_rate) (struct device *dev));
 u32 omap_device_get_context_loss_count(struct platform_device *pdev);
 
 /* Other */
diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
index a84e849..4cee430 100644
--- a/arch/arm/plat-omap/omap_device.c
+++ b/arch/arm/plat-omap/omap_device.c
@@ -810,6 +810,55 @@ int omap_device_enable_clocks(struct omap_device *od)
 	return 0;
 }
 
+int omap_device_set_rate(struct device *dev, unsigned long freq)
+{
+	struct platform_device *pdev;
+	struct omap_device *od;
+
+	pdev = container_of(dev, struct platform_device, dev);
+	od = _find_by_pdev(pdev);
+
+	if (!od->set_rate) {
+		dev_err(dev, "%s: No set_rate API for scaling device\n",
+			__func__);
+		return -ENODATA;
+	}
+
+	return od->set_rate(dev, freq);
+}
+
+unsigned long omap_device_get_rate(struct device *dev)
+{
+	struct platform_device *pdev;
+	struct omap_device *od;
+
+	pdev = container_of(dev, struct platform_device, dev);
+	od = _find_by_pdev(pdev);
+
+
+	if (!od->get_rate) {
+		dev_err(dev, "%s: No get rate API for the device\n",
+			__func__);
+		return 0;
+	}
+
+	return od->get_rate(dev);
+}
+
+void omap_device_register_dvfs_callbacks(struct device *dev,
+		int (*set_rate)(struct device *dev, unsigned long rate),
+		unsigned long (*get_rate) (struct device *dev))
+{
+	struct platform_device *pdev;
+	struct omap_device *od;
+
+	pdev = container_of(dev, struct platform_device, dev);
+	od = _find_by_pdev(pdev);
+
+	od->set_rate = set_rate;
+	od->get_rate = get_rate;
+}
+
 struct device omap_device_parent = {
 	.init_name	= "omap",
 	.parent         = &platform_bus,
-- 
1.7.0.4

[PATCH 03/13] OMAP: Implement Basic DVFS

[Vishwanath BS]

This patch introduces an API to perform DVFS for a given voltage domain.
It takes omap_vdd_dvfs_info pointer as input parameter, computes the highest
requested voltage for that vdd and scales all the devices in that vdd to the
requested frequency along with voltage scaling.

Based on original patch from Thara.

Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
Cc: Thara Gopinath <thara@ti.com>
---
 arch/arm/mach-omap2/dvfs.c |   87 +++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 86 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/dvfs.c b/arch/arm/mach-omap2/dvfs.c
index 8832e4a..cefc2be 100755
--- a/arch/arm/mach-omap2/dvfs.c
+++ b/arch/arm/mach-omap2/dvfs.c
@@ -21,7 +21,7 @@
 #include <plat/omap_device.h>
 
 /**
- * struct omap_dev_user_list - Structure maitain userlist per devide
+ * struct omap_dev_user_list - Structure maitain userlist per device
  *
  * @dev:       The device requesting for a particular frequency
  * @node:      The list head entry
@@ -413,6 +413,91 @@ static int omap_dvfs_remove_freq_request(struct omap_vdd_dvfs_info *dvfs_info,
 }
 
 /**
+ * omap_dvfs_voltage_scale() : API to scale the devices associated with a
+ *						voltage domain vdd voltage.
+ *
+ * @dvfs_info: omap_vdd_dvfs_info pointer for the required vdd
+ *
+ * This API runs through the list of devices associated with the
+ * voltage domain and scales the device rates to the one requested
+ * by the user or those corresponding to the new voltage of the
+ * voltage domain. Target voltage is the highest voltage in the vdd_user_list.
+ *
+ * Returns 0 on success
+ * else the error value.
+ */
+static int omap_dvfs_voltage_scale(struct omap_vdd_dvfs_info *dvfs_info)
+{
+	unsigned long curr_volt;
+	int is_volt_scaled = 0;
+	struct omap_vdd_dev_list *temp_dev;
+	struct plist_node *node;
+	int ret = 0;
+	struct voltagedomain *voltdm;
+	unsigned long volt;
+
+	if (!dvfs_info || IS_ERR(dvfs_info)) {
+		pr_warning("%s: VDD specified does not exist!\n", __func__);
+		return -EINVAL;
+	}
+
+	voltdm = dvfs_info->voltdm;
+
+	mutex_lock(&dvfs_info->scaling_mutex);
+
+	/* Find the highest voltage being requested */
+	node = plist_last(&dvfs_info->user_list);
+	volt = node->prio;
+
+	curr_volt = omap_voltage_get_nom_volt(voltdm);
+
+	if (curr_volt == volt) {
+		is_volt_scaled = 1;
+	} else if (curr_volt < volt) {
+		ret = omap_voltage_scale_vdd(voltdm, volt);
+		if (ret) {
+			pr_warning("%s: Unable to scale the %s to %ld volt\n",
+						__func__, voltdm->name, volt);
+			mutex_unlock(&dvfs_info->scaling_mutex);
+			return ret;
+		}
+		is_volt_scaled = 1;
+	}
+
+	list_for_each_entry(temp_dev, &dvfs_info->dev_list, node) {
+		struct device *dev;
+		struct opp *opp;
+		unsigned long freq;
+
+		dev = temp_dev->dev;
+		if (!plist_head_empty(&temp_dev->user_list)) {
+			node = plist_last(&temp_dev->user_list);
+			freq = node->prio;
+		} else {
+			opp = omap_dvfs_find_voltage(dev, volt);
+			if (IS_ERR(opp))
+				continue;
+			freq = opp_get_freq(opp);
+		}
+
+		if (freq == omap_device_get_rate(dev)) {
+			dev_dbg(dev, "%s: Already at the requested"
+				"rate %ld\n", __func__, freq);
+			continue;
+		}
+
+		ret |= omap_device_set_rate(dev, freq);
+	}
+
+	if (!is_volt_scaled && !ret)
+		omap_voltage_scale_vdd(voltdm, volt);
+
+	mutex_unlock(&dvfs_info->scaling_mutex);
+
+	return 0;
+}
+
+/**
  * omap_dvfs_init() - Initialize omap dvfs layer
  *
  * Initalizes omap dvfs layer. It basically allocates memory for
-- 
1.7.0.4

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

[Vishwanath BS]

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.

As part of this, some of the voltage layer structure definitions are moved from
voltage.c to voltage.h as it needs to be used in the dvfs layer for dependency
voltage handling.

Based on original patch from Thara.

Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
Cc: Thara Gopinath <thara@ti.com>
---
 arch/arm/mach-omap2/dvfs.c                |   87 +++++++++++++++++
 arch/arm/mach-omap2/voltage.c             |  117 -----------------------
 arch/arm/plat-omap/include/plat/voltage.h |  148 +++++++++++++++++++++++++++++
 3 files changed, 235 insertions(+), 117 deletions(-)

diff --git a/arch/arm/mach-omap2/dvfs.c b/arch/arm/mach-omap2/dvfs.c
index cefc2be..c9d3894 100755
--- a/arch/arm/mach-omap2/dvfs.c
+++ b/arch/arm/mach-omap2/dvfs.c
@@ -85,6 +85,7 @@ struct omap_vdd_dvfs_info {
 	struct mutex scaling_mutex; /* dvfs mutex */
 	struct voltagedomain *voltdm;
 	struct list_head dev_list;
+	struct device vdd_device;
 };
 
 static struct omap_vdd_dvfs_info *omap_dvfs_info_list;
@@ -98,6 +99,7 @@ static struct voltagedomain omap3_vdd[] = {
 	.name = "core",
 	},
 };
+static int omap_dvfs_voltage_scale(struct omap_vdd_dvfs_info *dvfs_info);
 
 static int __init omap_dvfs_init(void);
 
@@ -412,6 +414,79 @@ static int omap_dvfs_remove_freq_request(struct omap_vdd_dvfs_info *dvfs_info,
 	return ret;
 }
 
+/* Calculate dependency vdd voltage for given vdd voltage */
+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_lookup(dep_vdds[i].name);
+
+		act_volt = dep_volt;
+
+		/* See if dep_volt is possible for the vdd*/
+		ret = omap_dvfs_add_vdd_user(get_dvfs_info(dep_vdds[i].voltdm),
+				dev, act_volt);
+	}
+
+	return ret;
+}
+
+/* Scale dependent VDD */
+static int scale_dep_vdd(struct omap_vdd_dvfs_info *vdd_info)
+{
+	struct omap_vdd_dep_info *dep_vdds;
+	int i;
+	struct omap_vdd_info *main_vdd;
+	struct voltagedomain *voltdm = vdd_info->voltdm;
+	main_vdd = container_of(voltdm, struct omap_vdd_info, voltdm);
+
+	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_dvfs_voltage_scale(get_dvfs_info(dep_vdds[i].voltdm));
+
+	return 0;
+}
+
 /**
  * omap_dvfs_voltage_scale() : API to scale the devices associated with a
  *						voltage domain vdd voltage.
@@ -435,6 +510,7 @@ static int omap_dvfs_voltage_scale(struct omap_vdd_dvfs_info *dvfs_info)
 	int ret = 0;
 	struct voltagedomain *voltdm;
 	unsigned long volt;
+	struct omap_vdd_info *vdd;
 
 	if (!dvfs_info || IS_ERR(dvfs_info)) {
 		pr_warning("%s: VDD specified does not exist!\n", __func__);
@@ -442,6 +518,7 @@ static int omap_dvfs_voltage_scale(struct omap_vdd_dvfs_info *dvfs_info)
 	}
 
 	voltdm = dvfs_info->voltdm;
+	vdd = container_of(voltdm, struct omap_vdd_info, voltdm);
 
 	mutex_lock(&dvfs_info->scaling_mutex);
 
@@ -494,6 +571,16 @@ static int omap_dvfs_voltage_scale(struct omap_vdd_dvfs_info *dvfs_info)
 
 	mutex_unlock(&dvfs_info->scaling_mutex);
 
+	/* calculate the voltages for dependent vdd's */
+	if (calc_dep_vdd_volt(&dvfs_info->vdd_device, vdd, volt)) {
+		pr_warning("%s: Error in calculating dependent vdd voltages"
+			"for vdd_%s\n", __func__, voltdm->name);
+		return -EINVAL;
+	}
+
+	/* Scale dependent vdds */
+	scale_dep_vdd(dvfs_info);
+
 	return 0;
 }
 
diff --git a/arch/arm/mach-omap2/voltage.c b/arch/arm/mach-omap2/voltage.c
index ed6079c..92fe20d 100644
--- a/arch/arm/mach-omap2/voltage.c
+++ b/arch/arm/mach-omap2/voltage.c
@@ -39,123 +39,6 @@
 #define VP_TRANXDONE_TIMEOUT	300
 #define VOLTAGE_DIR_SIZE	16
 
-/* Voltage processor register offsets */
-struct vp_reg_offs {
-	u8 vpconfig;
-	u8 vstepmin;
-	u8 vstepmax;
-	u8 vlimitto;
-	u8 vstatus;
-	u8 voltage;
-};
-
-/* Voltage Processor bit field values, shifts and masks */
-struct vp_reg_val {
-	/* PRM module */
-	u16 prm_mod;
-	/* VPx_VPCONFIG */
-	u32 vpconfig_erroroffset;
-	u16 vpconfig_errorgain;
-	u32 vpconfig_errorgain_mask;
-	u8 vpconfig_errorgain_shift;
-	u32 vpconfig_initvoltage_mask;
-	u8 vpconfig_initvoltage_shift;
-	u32 vpconfig_timeouten;
-	u32 vpconfig_initvdd;
-	u32 vpconfig_forceupdate;
-	u32 vpconfig_vpenable;
-	/* VPx_VSTEPMIN */
-	u8 vstepmin_stepmin;
-	u16 vstepmin_smpswaittimemin;
-	u8 vstepmin_stepmin_shift;
-	u8 vstepmin_smpswaittimemin_shift;
-	/* VPx_VSTEPMAX */
-	u8 vstepmax_stepmax;
-	u16 vstepmax_smpswaittimemax;
-	u8 vstepmax_stepmax_shift;
-	u8 vstepmax_smpswaittimemax_shift;
-	/* VPx_VLIMITTO */
-	u8 vlimitto_vddmin;
-	u8 vlimitto_vddmax;
-	u16 vlimitto_timeout;
-	u8 vlimitto_vddmin_shift;
-	u8 vlimitto_vddmax_shift;
-	u8 vlimitto_timeout_shift;
-	/* PRM_IRQSTATUS*/
-	u32 tranxdone_status;
-};
-
-/* Voltage controller registers and offsets */
-struct vc_reg_info {
-	/* PRM module */
-	u16 prm_mod;
-	/* VC register offsets */
-	u8 smps_sa_reg;
-	u8 smps_volra_reg;
-	u8 bypass_val_reg;
-	u8 cmdval_reg;
-	u8 voltsetup_reg;
-	/*VC_SMPS_SA*/
-	u8 smps_sa_shift;
-	u32 smps_sa_mask;
-	/* VC_SMPS_VOL_RA */
-	u8 smps_volra_shift;
-	u32 smps_volra_mask;
-	/* VC_BYPASS_VAL */
-	u8 data_shift;
-	u8 slaveaddr_shift;
-	u8 regaddr_shift;
-	u32 valid;
-	/* VC_CMD_VAL */
-	u8 cmd_on_shift;
-	u8 cmd_onlp_shift;
-	u8 cmd_ret_shift;
-	u8 cmd_off_shift;
-	u32 cmd_on_mask;
-	/* PRM_VOLTSETUP */
-	u8 voltsetup_shift;
-	u32 voltsetup_mask;
-};
-
-/**
- * omap_vdd_info - Per Voltage Domain info
- *
- * @volt_data		: voltage table having the distinct voltages supported
- *			  by the domain and other associated per voltage data.
- * @pmic_info		: pmic specific parameters which should be populted by
- *			  the pmic drivers.
- * @vp_offs		: structure containing the offsets for various
- *			  vp registers
- * @vp_reg		: the register values, shifts, masks for various
- *			  vp registers
- * @vc_reg		: structure containing various various vc registers,
- *			  shifts, masks etc.
- * @voltdm		: pointer to the voltage domain structure
- * @debug_dir		: debug directory for this voltage domain.
- * @curr_volt		: current voltage for this vdd.
- * @ocp_mod		: The prm module for accessing the prm irqstatus reg.
- * @prm_irqst_reg	: prm irqstatus register.
- * @vp_enabled		: flag to keep track of whether vp is enabled or not
- * @volt_scale		: API to scale the voltage of the vdd.
- */
-struct omap_vdd_info {
-	struct omap_volt_data *volt_data;
-	struct omap_volt_pmic_info *pmic_info;
-	struct vp_reg_offs vp_offs;
-	struct vp_reg_val vp_reg;
-	struct vc_reg_info vc_reg;
-	struct voltagedomain voltdm;
-	struct dentry *debug_dir;
-	u32 curr_volt;
-	u16 ocp_mod;
-	u8 prm_irqst_reg;
-	bool vp_enabled;
-	u32 (*read_reg) (u16 mod, u8 offset);
-	void (*write_reg) (u32 val, u16 mod, u8 offset);
-	int (*volt_scale) (struct omap_vdd_info *vdd,
-		unsigned long target_volt);
-};
-
 static struct omap_vdd_info *vdd_info;
 /*
  * Number of scalable voltage domains.
diff --git a/arch/arm/plat-omap/include/plat/voltage.h b/arch/arm/plat-omap/include/plat/voltage.h
index 5bd204e..e0b7f22 100644
--- a/arch/arm/plat-omap/include/plat/voltage.h
+++ b/arch/arm/plat-omap/include/plat/voltage.h
@@ -113,6 +113,154 @@ struct omap_volt_pmic_info {
 	u8 (*uv_to_vsel) (unsigned long uV);
 };
 
+/* Voltage processor register offsets */
+struct vp_reg_offs {
+	u8 vpconfig;
+	u8 vstepmin;
+	u8 vstepmax;
+	u8 vlimitto;
+	u8 vstatus;
+	u8 voltage;
+};
+
+/* Voltage Processor bit field values, shifts and masks */
+struct vp_reg_val {
+	/* PRM module */
+	u16 prm_mod;
+	/* VPx_VPCONFIG */
+	u32 vpconfig_erroroffset;
+	u16 vpconfig_errorgain;
+	u32 vpconfig_errorgain_mask;
+	u8 vpconfig_errorgain_shift;
+	u32 vpconfig_initvoltage_mask;
+	u8 vpconfig_initvoltage_shift;
+	u32 vpconfig_timeouten;
+	u32 vpconfig_initvdd;
+	u32 vpconfig_forceupdate;
+	u32 vpconfig_vpenable;
+	/* VPx_VSTEPMIN */
+	u8 vstepmin_stepmin;
+	u16 vstepmin_smpswaittimemin;
+	u8 vstepmin_stepmin_shift;
+	u8 vstepmin_smpswaittimemin_shift;
+	/* VPx_VSTEPMAX */
+	u8 vstepmax_stepmax;
+	u16 vstepmax_smpswaittimemax;
+	u8 vstepmax_stepmax_shift;
+	u8 vstepmax_smpswaittimemax_shift;
+	/* VPx_VLIMITTO */
+	u8 vlimitto_vddmin;
+	u8 vlimitto_vddmax;
+	u16 vlimitto_timeout;
+	u8 vlimitto_vddmin_shift;
+	u8 vlimitto_vddmax_shift;
+	u8 vlimitto_timeout_shift;
+	/* PRM_IRQSTATUS*/
+	u32 tranxdone_status;
+};
+
+/* Voltage controller registers and offsets */
+struct vc_reg_info {
+	/* PRM module */
+	u16 prm_mod;
+	/* VC register offsets */
+	u8 smps_sa_reg;
+	u8 smps_volra_reg;
+	u8 bypass_val_reg;
+	u8 cmdval_reg;
+	u8 voltsetup_reg;
+	/*VC_SMPS_SA*/
+	u8 smps_sa_shift;
+	u32 smps_sa_mask;
+	/* VC_SMPS_VOL_RA */
+	u8 smps_volra_shift;
+	u32 smps_volra_mask;
+	/* VC_BYPASS_VAL */
+	u8 data_shift;
+	u8 slaveaddr_shift;
+	u8 regaddr_shift;
+	u32 valid;
+	/* VC_CMD_VAL */
+	u8 cmd_on_shift;
+	u8 cmd_onlp_shift;
+	u8 cmd_ret_shift;
+	u8 cmd_off_shift;
+	u32 cmd_on_mask;
+	/* PRM_VOLTSETUP */
+	u8 voltsetup_shift;
+	u32 voltsetup_mask;
+};
+
+
+/**
+ * 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.
+ */
+struct omap_vdd_dep_info {
+	char *name;
+	struct voltagedomain *voltdm;
+	struct omap_vdd_dep_volt *dep_table;
+};
+
+
+/**
+ * omap_vdd_info - Per Voltage Domain info
+ *
+ * @volt_data		: voltage table having the distinct voltages supported
+ *			  by the domain and other associated per voltage data.
+ * @pmic_info		: pmic specific parameters which should be populted by
+ *			  the pmic drivers.
+ * @vp_offs		: structure containing the offsets for various
+ *			  vp registers
+ * @vp_reg		: the register values, shifts, masks for various
+ *			  vp registers
+ * @vc_reg		: structure containing various various vc registers,
+ *			  shifts, masks etc.
+ * @voltdm		: pointer to the voltage domain structure
+ * @debug_dir		: debug directory for this voltage domain.
+ * @curr_volt		: current voltage for this vdd.
+ * @ocp_mod		: The prm module for accessing the prm irqstatus reg.
+ * @prm_irqst_reg	: prm irqstatus register.
+ * @vp_enabled		: flag to keep track of whether vp is enabled or not
+ * @volt_scale		: API to scale the voltage of the vdd.
+ */
+struct omap_vdd_info {
+	struct omap_volt_data *volt_data;
+	struct omap_volt_pmic_info *pmic_info;
+	struct vp_reg_offs vp_offs;
+	struct vp_reg_val vp_reg;
+	struct vc_reg_info vc_reg;
+	struct voltagedomain voltdm;
+	struct omap_vdd_dep_info *dep_vdd_info;
+	int nr_dep_vdd;
+	struct dentry *debug_dir;
+	u32 curr_volt;
+	u16 ocp_mod;
+	u8 prm_irqst_reg;
+	bool vp_enabled;
+	u32 (*read_reg) (u16 mod, u8 offset);
+	void (*write_reg) (u32 val, u16 mod, u8 offset);
+	int (*volt_scale) (struct omap_vdd_info *vdd,
+		unsigned long target_volt);
+};
+
 unsigned long omap_vp_get_curr_volt(struct voltagedomain *voltdm);
 void omap_vp_enable(struct voltagedomain *voltdm);
 void omap_vp_disable(struct voltagedomain *voltdm);
-- 
1.7.0.4

[PATCH 05/13] OMAP: Introduce device scale implementation

[Vishwanath BS]

This patch adds omap_device_scale API  which can be used to generic
device rate scaling.

Based on original patch from Thara.

Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
Cc: Thara Gopinath <thara@ti.com>
---
 arch/arm/mach-omap2/dvfs.c             |  116 ++++++++++++++++++++++++++++++++
 arch/arm/plat-omap/include/plat/dvfs.h |    7 ++
 2 files changed, 123 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/dvfs.c b/arch/arm/mach-omap2/dvfs.c
index c9d3894..05a9ce3 100755
--- a/arch/arm/mach-omap2/dvfs.c
+++ b/arch/arm/mach-omap2/dvfs.c
@@ -19,6 +19,7 @@
 #include <plat/common.h>
 #include <plat/voltage.h>
 #include <plat/omap_device.h>
+#include <plat/smartreflex.h>
 
 /**
  * struct omap_dev_user_list - Structure maitain userlist per device
@@ -585,6 +586,121 @@ static int omap_dvfs_voltage_scale(struct omap_vdd_dvfs_info *dvfs_info)
 }
 
 /**
+ * omap_device_scale() - Set a new rate at which the device is to operate
+ * @req_dev:	pointer to the device requesting the scaling.
+ * @target_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_scale(struct device *req_dev, struct device *target_dev,
+			unsigned long rate)
+{
+	struct opp *opp;
+	unsigned long volt, freq, min_freq, max_freq;
+	struct omap_vdd_dvfs_info *dvfs_info;
+	struct platform_device *pdev;
+	struct omap_device *od;
+	int ret = 0;
+
+	pdev = container_of(target_dev, struct platform_device, dev);
+	od = container_of(pdev, struct omap_device, pdev);
+
+	/*
+	 * Figure out if the desired frquency lies between the
+	 * maximum and minimum possible for the particular device
+	 */
+	min_freq = 0;
+	if (IS_ERR(opp_find_freq_ceil(target_dev, &min_freq))) {
+		dev_err(target_dev, "%s: Unable to find lowest opp\n",
+						__func__);
+		return -ENODEV;
+	}
+
+	max_freq = ULONG_MAX;
+	if (IS_ERR(opp_find_freq_floor(target_dev, &max_freq))) {
+		dev_err(target_dev, "%s: Unable to find highest opp\n",
+						__func__);
+		return -ENODEV;
+	}
+
+	if (rate < min_freq)
+		freq = min_freq;
+	else if (rate > max_freq)
+		freq = max_freq;
+	else
+		freq = rate;
+
+	opp = opp_find_freq_ceil(target_dev, &freq);
+	if (IS_ERR(opp)) {
+		dev_err(target_dev, "%s: Unable to find OPP for freq%ld\n",
+			__func__, rate);
+		return -ENODEV;
+	}
+
+	/* 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.
+	 */
+
+	if (rate) {
+		dvfs_info = get_dvfs_info(od->hwmods[0]->voltdm);
+
+		ret = omap_dvfs_add_freq_request(dvfs_info, req_dev,
+						target_dev, freq);
+		if (ret) {
+			dev_err(target_dev, "%s: Unable to add frequency request\n",
+				__func__);
+			return ret;
+		}
+
+		ret = omap_dvfs_add_vdd_user(dvfs_info, req_dev, volt);
+		if (ret) {
+			dev_err(target_dev, "%s: Unable to add voltage request\n",
+				__func__);
+			omap_dvfs_remove_freq_request(dvfs_info, req_dev,
+				target_dev);
+			return ret;
+		}
+	} else {
+		dvfs_info = get_dvfs_info(od->hwmods[0]->voltdm);
+
+		ret = omap_dvfs_remove_freq_request(dvfs_info, req_dev,
+				target_dev);
+		if (ret) {
+			dev_err(target_dev, "%s: Unable to remove frequency request\n",
+				__func__);
+			return ret;
+		}
+
+		ret = omap_dvfs_remove_vdd_user(dvfs_info, req_dev);
+		if (ret) {
+			dev_err(target_dev, "%s: Unable to remove voltage request\n",
+				__func__);
+			return ret;
+		}
+	}
+
+	/* Do the actual scaling */
+	ret = omap_dvfs_voltage_scale(dvfs_info);
+	if (!ret)
+		if (omap_device_get_rate(target_dev) >= rate)
+			return 0;
+
+	return ret;
+}
+EXPORT_SYMBOL(omap_device_scale);
+
+/**
  * omap_dvfs_init() - Initialize omap dvfs layer
  *
  * Initalizes omap dvfs layer. It basically allocates memory for
diff --git a/arch/arm/plat-omap/include/plat/dvfs.h b/arch/arm/plat-omap/include/plat/dvfs.h
index 1302990..1be2b9d 100644
--- a/arch/arm/plat-omap/include/plat/dvfs.h
+++ b/arch/arm/plat-omap/include/plat/dvfs.h
@@ -17,11 +17,18 @@
 
 #ifdef CONFIG_PM
 int omap_dvfs_register_device(struct voltagedomain *voltdm, struct device *dev);
+int omap_device_scale(struct device *req_dev, struct device *dev,
+			unsigned long rate);
 #else
 static inline int omap_dvfs_register_device(struct voltagedomain *voltdm,
 		struct device *dev)
 {
 	return -EINVAL;
 }
+static inline int omap_device_scale(struct device *req_dev, struct devices
+			*target_dev, unsigned long rate);
+{
+	return -EINVAL;
+}
 #endif
 #endif
-- 
1.7.0.4

[PATCH 06/13] OMAP: Disable Smartreflex across DVFS

[Vishwanath BS]

From: Thara Gopinath <thara@ti.com>

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>
Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
---
 arch/arm/mach-omap2/dvfs.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/dvfs.c b/arch/arm/mach-omap2/dvfs.c
index 05a9ce3..1e5492c 100755
--- a/arch/arm/mach-omap2/dvfs.c
+++ b/arch/arm/mach-omap2/dvfs.c
@@ -529,6 +529,9 @@ static int omap_dvfs_voltage_scale(struct omap_vdd_dvfs_info *dvfs_info)
 
 	curr_volt = omap_voltage_get_nom_volt(voltdm);
 
+	/* Disable smartreflex module across voltage and frequency scaling */
+	omap_sr_disable(voltdm);
+
 	if (curr_volt == volt) {
 		is_volt_scaled = 1;
 	} else if (curr_volt < volt) {
@@ -536,6 +539,7 @@ static int omap_dvfs_voltage_scale(struct omap_vdd_dvfs_info *dvfs_info)
 		if (ret) {
 			pr_warning("%s: Unable to scale the %s to %ld volt\n",
 						__func__, voltdm->name, volt);
+			omap_sr_enable(voltdm);
 			mutex_unlock(&dvfs_info->scaling_mutex);
 			return ret;
 		}
@@ -570,6 +574,9 @@ static int omap_dvfs_voltage_scale(struct omap_vdd_dvfs_info *dvfs_info)
 	if (!is_volt_scaled && !ret)
 		omap_voltage_scale_vdd(voltdm, volt);
 
+	/* Enable Smartreflex module */
+	omap_sr_enable(voltdm);
+
 	mutex_unlock(&dvfs_info->scaling_mutex);
 
 	/* calculate the voltages for dependent vdd's */
-- 
1.7.0.4

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

[Vishwanath BS]

From: Thara Gopinath <thara@ti.com>

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 omap_device_populate_rate_fns during system init to register
various set_rate and get_rate APIs with the omap device layer

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

diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
index d5a102c..1b94ad4 100644
--- a/arch/arm/mach-omap2/pm.c
+++ b/arch/arm/mach-omap2/pm.c
@@ -23,6 +23,9 @@
 #include "powerdomain.h"
 #include "clockdomain.h"
 #include "pm.h"
+#include "cm2xxx_3xxx.h"
+#include "cm-regbits-34xx.h"
+#include "prm.h"
 
 static struct omap_device_pm_latency *pm_lats;
 
@@ -31,6 +34,8 @@ static struct device *iva_dev;
 static struct device *l3_dev;
 static struct device *dsp_dev;
 
+static struct clk *dpll1_clk, *dpll2_clk, *dpll3_clk;
+
 struct device *omap2_get_mpuss_device(void)
 {
 	WARN_ON_ONCE(!mpu_dev);
@@ -77,6 +82,55 @@ static int _init_omap_device(char *name, struct device **new_dev)
 	return 0;
 }
 
+static unsigned long omap3_mpu_get_rate(struct device *dev)
+{
+	return dpll1_clk->rate;
+}
+
+static int omap3_mpu_set_rate(struct device *dev, unsigned long rate)
+{
+	int ret;
+
+	ret = clk_set_rate(dpll1_clk, rate);
+	if (ret) {
+		dev_warn(dev, "%s: Unable to set rate to %ld\n",
+			__func__, rate);
+		return ret;
+	}
+
+	return 0;
+}
+
+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 = omap2_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 = omap2_cm_read_mod_reg(CORE_MOD, CM_CLKSEL) &
+			OMAP3430_CLKSEL_L3_MASK;
+	return dpll3_clk->rate / l3_div;
+}
+
+
 /*
  * Build omap_devices for processors and bus.
  */
@@ -90,6 +144,23 @@ static void omap2_init_processor_devices(void)
 	} else {
 		_init_omap_device("l3_main", &l3_dev);
 	}
+
+	if (cpu_is_omap34xx()) {
+		dpll1_clk = clk_get(NULL, "dpll1_ck");
+		dpll2_clk = clk_get(NULL, "dpll2_ck");
+		dpll3_clk = clk_get(NULL, "dpll3_m2_ck");
+
+		if (mpu_dev)
+			omap_device_register_dvfs_callbacks(mpu_dev,
+				omap3_mpu_set_rate, omap3_mpu_get_rate);
+		if (iva_dev)
+			omap_device_register_dvfs_callbacks(iva_dev,
+				omap3_iva_set_rate, omap3_iva_get_rate);
+		if (l3_dev)
+			omap_device_register_dvfs_callbacks(l3_dev,
+				omap3_l3_set_rate, omap3_l3_get_rate);
+
+	}
 }
 
 /* Types of sleep_switch used in omap_set_pwrdm_state */
-- 
1.7.0.4

[PATCH 08/13] OMAP3: cpufreq driver changes for DVFS support

[Vishwanath BS]

Changes in the omap cpufreq driver for DVFS support.

Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
---
 arch/arm/plat-omap/cpu-omap.c |   35 ++++++++++++++++++++++++++++++++---
 1 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/arch/arm/plat-omap/cpu-omap.c b/arch/arm/plat-omap/cpu-omap.c
index 1c1b80b..d965220 100644
--- a/arch/arm/plat-omap/cpu-omap.c
+++ b/arch/arm/plat-omap/cpu-omap.c
@@ -30,10 +30,12 @@
 #include <mach/hardware.h>
 #include <plat/clock.h>
 #include <asm/system.h>
+#include <asm/cpu.h>
 
 #if defined(CONFIG_ARCH_OMAP3) && !defined(CONFIG_OMAP_PM_NONE)
 #include <plat/omap-pm.h>
 #include <plat/common.h>
+#include <plat/dvfs.h>
 #endif
 
 #define VERY_HI_RATE	900000000
@@ -85,11 +87,11 @@ static int omap_target(struct cpufreq_policy *policy,
 		       unsigned int target_freq,
 		       unsigned int relation)
 {
-#ifdef CONFIG_ARCH_OMAP1
 	struct cpufreq_freqs freqs;
-#endif
 #if defined(CONFIG_ARCH_OMAP3) && !defined(CONFIG_OMAP_PM_NONE)
 	unsigned long freq;
+	int i;
+	struct cpufreq_freqs freqs_notify;
 	struct device *mpu_dev = omap2_get_mpuss_device();
 #endif
 	int ret = 0;
@@ -116,9 +118,36 @@ static int omap_target(struct cpufreq_policy *policy,
 	ret = clk_set_rate(mpu_clk, freqs.new * 1000);
 	cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
 #elif defined(CONFIG_ARCH_OMAP3) && !defined(CONFIG_OMAP_PM_NONE)
+	freqs.old = omap_getspeed(policy->cpu);;
+	freqs_notify.new = clk_round_rate(mpu_clk, target_freq * 1000) / 1000;
+	freqs.cpu = policy->cpu;
+
+	if (freqs.old == freqs.new)
+		return ret;
+
+	/* pre notifiers */
+	for_each_cpu(i, policy->cpus) {
+		freqs.cpu = i;
+		cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
+	}
+
+	/* scale the frequency */
 	freq = target_freq * 1000;
 	if (opp_find_freq_ceil(mpu_dev, &freq))
-		omap_pm_cpu_set_freq(freq);
+		omap_device_scale(mpu_dev, mpu_dev, freq);
+
+	/* Update loops per jiffy */
+	freqs.new = omap_getspeed(policy->cpu);
+	for_each_cpu(i, policy->cpus)
+		per_cpu(cpu_data, i).loops_per_jiffy =
+		cpufreq_scale(per_cpu(cpu_data, i).loops_per_jiffy,
+				freqs.old, freqs.new);
+
+	/* post notifiers */
+	for_each_cpu(i, policy->cpus) {
+		freqs.cpu = i;
+		cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
+	}
 #endif
 	return ret;
 }
-- 
1.7.0.4

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

[Vishwanath BS]

From: Thara Gopinath <thara@ti.com>

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 8d81813..c57f34d 100644
--- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
@@ -94,6 +94,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,
@@ -384,6 +385,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),
@@ -412,6 +414,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.0.4

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

[Vishwanath BS]

From: Thara Gopinath <thara@ti.com>

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>

This patch has checkpatch warnings for line over 80 chars. It is not fixed for
code readability.
---
 arch/arm/mach-omap2/voltage.c |   42 +++++++++++++++++++++++++++++++++++++++-
 1 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/voltage.c b/arch/arm/mach-omap2/voltage.c
index 92fe20d..8881c0c 100644
--- a/arch/arm/mach-omap2/voltage.c
+++ b/arch/arm/mach-omap2/voltage.c
@@ -191,6 +191,39 @@ static struct omap_volt_data omap44xx_vdd_core_volt_data[] = {
 	VOLT_DATA_DEFINE(0, 0, 0, 0),
 };
 
+/* OMAP 3430 MPU Core VDD dependency table */
+static struct omap_vdd_dep_volt omap34xx_vdd1_vdd2_data[] = {
+	{.main_vdd_volt = OMAP3430_VDD_MPU_OPP1_UV, .dep_vdd_volt = OMAP4430_VDD_CORE_OPP50_UV},
+	{.main_vdd_volt = OMAP3430_VDD_MPU_OPP2_UV, .dep_vdd_volt = OMAP4430_VDD_CORE_OPP50_UV},
+	{.main_vdd_volt = OMAP3430_VDD_MPU_OPP3_UV, .dep_vdd_volt = OMAP4430_VDD_CORE_OPP100_UV},
+	{.main_vdd_volt = OMAP3430_VDD_MPU_OPP4_UV, .dep_vdd_volt = OMAP4430_VDD_CORE_OPP100_UV},
+	{.main_vdd_volt = OMAP3430_VDD_MPU_OPP5_UV, .dep_vdd_volt = OMAP4430_VDD_CORE_OPP100_UV},
+	{.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,
+	},
+};
+
+/* OMAP 3630 MPU Core VDD dependency table */
+static struct omap_vdd_dep_volt omap36xx_vdd1_vdd2_data[] = {
+	{.main_vdd_volt = OMAP3630_VDD_MPU_OPP50_UV, .dep_vdd_volt = OMAP3630_VDD_CORE_OPP50_UV},
+	{.main_vdd_volt = OMAP3630_VDD_MPU_OPP100_UV, .dep_vdd_volt = OMAP3630_VDD_CORE_OPP100_UV},
+	{.main_vdd_volt = OMAP3630_VDD_MPU_OPP120_UV, .dep_vdd_volt = OMAP3630_VDD_CORE_OPP100_UV},
+	{.main_vdd_volt = OMAP3630_VDD_MPU_OPP1G_UV, .dep_vdd_volt = OMAP3630_VDD_CORE_OPP100_UV},
+	{.main_vdd_volt = 0, .dep_vdd_volt = 0},
+};
+
+static struct omap_vdd_dep_info omap36xx_vdd1_dep_info[] = {
+	{
+		.name	= "core",
+		.dep_table = omap36xx_vdd1_vdd2_data,
+	},
+};
+
 static struct dentry *voltage_dir;
 
 /* Init function pointers */
@@ -696,10 +729,15 @@ static int __init omap3_vdd_data_configure(struct omap_vdd_info *vdd)
 	}
 
 	if (!strcmp(vdd->voltdm.name, "mpu")) {
-		if (cpu_is_omap3630())
+		if (cpu_is_omap3630()) {
 			vdd->volt_data = omap36xx_vddmpu_volt_data;
-		else
+			vdd->dep_vdd_info = omap36xx_vdd1_dep_info;
+			vdd->nr_dep_vdd = ARRAY_SIZE(omap36xx_vdd1_dep_info);
+		} else {
 			vdd->volt_data = omap34xx_vddmpu_volt_data;
+			vdd->dep_vdd_info = omap34xx_vdd1_dep_info;
+			vdd->nr_dep_vdd = ARRAY_SIZE(omap34xx_vdd1_dep_info);
+		}
 
 		vdd->vp_reg.tranxdone_status = OMAP3430_VP1_TRANXDONE_ST_MASK;
 		vdd->vc_reg.cmdval_reg = OMAP3_PRM_VC_CMD_VAL_0_OFFSET;
-- 
1.7.0.4

[PATCH 11/13] OMAP2PLUS: Replace voltage values with Macros

[Vishwanath BS]

Currently voltage values on opp tables are hardcoded. As these voltage values
are anyway defined in voltage.h as macros, opp table can reuse these values.
This will avoid opp table and voltage layer having conflicting values.

Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
This patch has 2 line over 80 char warning. This is kept for readability
purpose.
---
 arch/arm/mach-omap2/opp3xxx_data.c |   47 ++++++++++++++++++-----------------
 arch/arm/mach-omap2/opp4xxx_data.c |   13 +++++----
 2 files changed, 31 insertions(+), 29 deletions(-)

diff --git a/arch/arm/mach-omap2/opp3xxx_data.c b/arch/arm/mach-omap2/opp3xxx_data.c
index 0486fce..eefd6af 100644
--- a/arch/arm/mach-omap2/opp3xxx_data.c
+++ b/arch/arm/mach-omap2/opp3xxx_data.c
@@ -19,20 +19,21 @@
 #include <linux/module.h>
 
 #include <plat/cpu.h>
+#include <plat/voltage.h>
 
 #include "omap_opp_data.h"
 
 static struct omap_opp_def __initdata omap34xx_opp_def_list[] = {
 	/* MPU OPP1 */
-	OPP_INITIALIZER("mpu", true, 125000000, 975000),
+	OPP_INITIALIZER("mpu", true, 125000000, OMAP3430_VDD_MPU_OPP1_UV),
 	/* MPU OPP2 */
-	OPP_INITIALIZER("mpu", true, 250000000, 1075000),
+	OPP_INITIALIZER("mpu", true, 250000000, OMAP3430_VDD_MPU_OPP2_UV),
 	/* MPU OPP3 */
-	OPP_INITIALIZER("mpu", true, 500000000, 1200000),
+	OPP_INITIALIZER("mpu", true, 500000000, OMAP3430_VDD_MPU_OPP3_UV),
 	/* MPU OPP4 */
-	OPP_INITIALIZER("mpu", true, 550000000, 1270000),
+	OPP_INITIALIZER("mpu", true, 550000000, OMAP3430_VDD_MPU_OPP4_UV),
 	/* MPU OPP5 */
-	OPP_INITIALIZER("mpu", true, 600000000, 1350000),
+	OPP_INITIALIZER("mpu", true, 600000000, OMAP3430_VDD_MPU_OPP5_UV),
 
 	/*
 	 * L3 OPP1 - 41.5 MHz is disabled because: The voltage for that OPP is
@@ -42,47 +43,47 @@ static struct omap_opp_def __initdata omap34xx_opp_def_list[] = {
 	 * impact that frequency will do to the MPU and the whole system in
 	 * general.
 	 */
-	OPP_INITIALIZER("l3_main", false, 41500000, 975000),
+	OPP_INITIALIZER("l3_main", false, 41500000, OMAP3430_VDD_CORE_OPP1_UV),
 	/* L3 OPP2 */
-	OPP_INITIALIZER("l3_main", true, 83000000, 1050000),
+	OPP_INITIALIZER("l3_main", true, 83000000, OMAP3430_VDD_CORE_OPP2_UV),
 	/* L3 OPP3 */
-	OPP_INITIALIZER("l3_main", true, 166000000, 1150000),
+	OPP_INITIALIZER("l3_main", true, 166000000, OMAP3430_VDD_CORE_OPP3_UV),
 
 	/* DSP OPP1 */
-	OPP_INITIALIZER("iva", true, 90000000, 975000),
+	OPP_INITIALIZER("iva", true, 90000000, OMAP3430_VDD_MPU_OPP1_UV),
 	/* DSP OPP2 */
-	OPP_INITIALIZER("iva", true, 180000000, 1075000),
+	OPP_INITIALIZER("iva", true, 180000000, OMAP3430_VDD_MPU_OPP2_UV),
 	/* DSP OPP3 */
-	OPP_INITIALIZER("iva", true, 360000000, 1200000),
+	OPP_INITIALIZER("iva", true, 360000000, OMAP3430_VDD_MPU_OPP3_UV),
 	/* DSP OPP4 */
-	OPP_INITIALIZER("iva", true, 400000000, 1270000),
+	OPP_INITIALIZER("iva", true, 400000000, OMAP3430_VDD_MPU_OPP4_UV),
 	/* DSP OPP5 */
-	OPP_INITIALIZER("iva", true, 430000000, 1350000),
+	OPP_INITIALIZER("iva", true, 430000000, OMAP3430_VDD_MPU_OPP5_UV),
 };
 
 static struct omap_opp_def __initdata omap36xx_opp_def_list[] = {
 	/* MPU OPP1 - OPP50 */
-	OPP_INITIALIZER("mpu", true,  300000000, 1012500),
+	OPP_INITIALIZER("mpu", true,  300000000, OMAP3630_VDD_MPU_OPP50_UV),
 	/* MPU OPP2 - OPP100 */
-	OPP_INITIALIZER("mpu", true,  600000000, 1200000),
+	OPP_INITIALIZER("mpu", true,  600000000, OMAP3630_VDD_MPU_OPP100_UV),
 	/* MPU OPP3 - OPP-Turbo */
-	OPP_INITIALIZER("mpu", false, 800000000, 1325000),
+	OPP_INITIALIZER("mpu", false, 800000000, OMAP3630_VDD_MPU_OPP120_UV),
 	/* MPU OPP4 - OPP-SB */
-	OPP_INITIALIZER("mpu", false, 1000000000, 1375000),
+	OPP_INITIALIZER("mpu", false, 1000000000, OMAP3630_VDD_MPU_OPP1G_UV),
 
 	/* L3 OPP1 - OPP50 */
-	OPP_INITIALIZER("l3_main", true, 100000000, 1000000),
+	OPP_INITIALIZER("l3_main", true, 100000000, OMAP3630_VDD_CORE_OPP50_UV),
 	/* L3 OPP2 - OPP100, OPP-Turbo, OPP-SB */
-	OPP_INITIALIZER("l3_main", true, 200000000, 1200000),
+	OPP_INITIALIZER("l3_main", true, 200000000, OMAP3630_VDD_CORE_OPP100_UV),
 
 	/* DSP OPP1 - OPP50 */
-	OPP_INITIALIZER("iva", true,  260000000, 1012500),
+	OPP_INITIALIZER("iva", true,  260000000, OMAP3630_VDD_MPU_OPP50_UV),
 	/* DSP OPP2 - OPP100 */
-	OPP_INITIALIZER("iva", true,  520000000, 1200000),
+	OPP_INITIALIZER("iva", true,  520000000, OMAP3630_VDD_MPU_OPP100_UV),
 	/* DSP OPP3 - OPP-Turbo */
-	OPP_INITIALIZER("iva", false, 660000000, 1325000),
+	OPP_INITIALIZER("iva", false, 660000000, OMAP3630_VDD_MPU_OPP120_UV),
 	/* DSP OPP4 - OPP-SB */
-	OPP_INITIALIZER("iva", false, 800000000, 1375000),
+	OPP_INITIALIZER("iva", false, 800000000, OMAP3630_VDD_MPU_OPP1G_UV),
 };
 
 /**
diff --git a/arch/arm/mach-omap2/opp4xxx_data.c b/arch/arm/mach-omap2/opp4xxx_data.c
index a11fa56..80d08bc 100644
--- a/arch/arm/mach-omap2/opp4xxx_data.c
+++ b/arch/arm/mach-omap2/opp4xxx_data.c
@@ -20,22 +20,23 @@
 #include <linux/module.h>
 
 #include <plat/cpu.h>
+#include <plat/voltage.h>
 
 #include "omap_opp_data.h"
 
 static struct omap_opp_def __initdata omap44xx_opp_def_list[] = {
 	/* MPU OPP1 - OPP50 */
-	OPP_INITIALIZER("mpu", true, 300000000, 1100000),
+	OPP_INITIALIZER("mpu", true, 300000000, OMAP4430_VDD_MPU_OPP50_UV),
 	/* MPU OPP2 - OPP100 */
-	OPP_INITIALIZER("mpu", true, 600000000, 1200000),
+	OPP_INITIALIZER("mpu", true, 600000000, OMAP4430_VDD_MPU_OPP100_UV),
 	/* MPU OPP3 - OPP-Turbo */
-	OPP_INITIALIZER("mpu", false, 800000000, 1260000),
+	OPP_INITIALIZER("mpu", false, 800000000, OMAP4430_VDD_MPU_OPPTURBO_UV),
 	/* MPU OPP4 - OPP-SB */
-	OPP_INITIALIZER("mpu", false, 1008000000, 1350000),
+	OPP_INITIALIZER("mpu", false, 1008000000, OMAP4430_VDD_MPU_OPPNITRO_UV),
 	/* L3 OPP1 - OPP50 */
-	OPP_INITIALIZER("l3_main_1", true, 100000000, 930000),
+	OPP_INITIALIZER("l3_main_1", true, 100000000, OMAP4430_VDD_CORE_OPP50_UV),
 	/* L3 OPP2 - OPP100, OPP-Turbo, OPP-SB */
-	OPP_INITIALIZER("l3_main_1", true, 200000000, 1100000),
+	OPP_INITIALIZER("l3_main_1", true, 200000000, OMAP4430_VDD_CORE_OPP100_UV),
 	/* TODO: add IVA, DSP, aess, fdif, gpu */
 };
 
-- 
1.7.0.4

[PATCH 12/13] OMAP2PLUS: Enable various options in defconfig

[Vishwanath BS]

From: Thara Gopinath <thara@ti.com>

This patch enables Smartreflex and Cpu Freq in the
omap2plus defconfig.

Signed-off-by: Thara Gopinath <thara@ti.com>
---
 arch/arm/configs/omap2plus_defconfig |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/arch/arm/configs/omap2plus_defconfig b/arch/arm/configs/omap2plus_defconfig
index ccedde1..b7888ae 100644
--- a/arch/arm/configs/omap2plus_defconfig
+++ b/arch/arm/configs/omap2plus_defconfig
@@ -291,3 +291,7 @@ CONFIG_CRC_T10DIF=y
 CONFIG_CRC_ITU_T=y
 CONFIG_CRC7=y
 CONFIG_LIBCRC32C=y
+CONFIG_CPU_FREQ=y
+CONFIG_CPU_FREQ_GOV_USERSPACE=y
+CONFIG_OMAP_SMARTREFLEX=y
+CONFIG_OMAP_SMARTREFLEX_CLASS3=y
-- 
1.7.0.4

[PATCH 13/13] OMAP: Add DVFS Documentation

[Vishwanath BS]

Add Documentation for DVFS Framework

Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
---
 Documentation/arm/OMAP/omap_dvfs |  111 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 111 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/arm/OMAP/omap_dvfs

diff --git a/Documentation/arm/OMAP/omap_dvfs b/Documentation/arm/OMAP/omap_dvfs
new file mode 100644
index 0000000..b026643
--- /dev/null
+++ b/Documentation/arm/OMAP/omap_dvfs
@@ -0,0 +1,111 @@
+*=============*
+* DVFS Framework *
+*=============*
+(C) 2011 Vishwnath BS <vishwanath.bs@ti.com>, Texas Instruments Incorporated
+Contents
+--------
+1. Introduction
+2. Data Structure Organization
+3. DVFS APIs
+
+1. Introduction
+===============
+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 takes 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. Keep track of frequency requests for each of the device. This will enable
+   to scale individual devices to different frequency (even w/o scaling voltage
+   aka frequency throttling)
+5. Generic dvfs API that can be called by anybody to scale a device opp.
+   This API takes the device pointer and frequency to which the device
+   needs to be scaled to. This API then internally finds 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 he device opp table. Then this API will add requested frequency into
+   the corresponding target device frequency list and add voltage request to
+   the corresponding vdd. Subsequently it calls voltage scale function which
+   will find out the highest requested voltage for the given vdd and scales
+   the voltage to the required one. It also runs 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.
+6. Handle inter VDD dependecies.
+
+
+2.  The Core DVFS data structure:
+=================================
+
+ 					|-------------------|			|-------------------|
+ 					|User2 (dev2, freq2)|			|User4 (dev4, freq4)|
+ 					|-------^-----------|			|-------^-----------|
+ 							|								|
+ 					|-------|-----------|			|-------|-----------|
+ 					|User1 (dev1, freq1)|			|User3 (dev3, freq3)|(omap_dev_user_list)
+ 					|-------^-----------|			|-------^-----------|
+                            |                               |
+ 						|---|--------------|             |------------------|
+ 		     |--------->| DEV1 (dev)       |------------>| DEV2 (dev)       |(omap_vdd_dev_list)
+ 			 |			|omap_dev_user_list|			 |omap_dev_user_list|
+ 			 |			|------------------|             |------------------|
+ 			 |
+   |---------|-----------|
+   |       VDD_n         |
+   |  omap_vdd_dev_list  |
+   |  omap_vdd_user_list |(omap_vdd_dvfs_info)
+   |                     |
+   |--------|------------|
+ 			|
+            |           |------------|  |------------|  |--------------|
+            |---------> | vdd_user1  |->|  vdd_user2 |->|   vdd_user3  | (omap_vdd_user_list)
+ 						| (dev, volt)|	| (dev, volt)|  | (dev, volt)  |
+ 						|------------|	|------------|	|--------------|
+
+3.  APIs:
+ =====
+ 1. omap_device_scale - Set a new rate at which the device is to operate
+
+  Examples:
+  1. Simple Device scaling:
+  Suppose module M wants to put device dev1 to frequency f1. Let's say that mdev
+  is the device * for module M. Then this could be achieved by
+  ret = omap_device_scale(mdev, dev1, f1)
+  if (ret)
+  	/* handle error *.
+
+  2. Frequency Throttling
+  Suppose say there are 2 modules M1 and M2 in Voltage domain VDDx.
+  Module M1 wants to set VDDx to OPP100 voltage which means M1 and M2 will
+  be running at OPP100 frequency. Suppose Module M2 wants to run at OPP50
+  frequency (say f2_opp50) instead of OPP100. This can be achieved by
+
+  /* this operation will place M1 and M2 to run at OPP100 */
+  ret = omap_device_scale(mdev1, dev1, f1_opp100);
+  if (ret)
+  	/* handle error *.
+
+  /* This operation will bring M2 to run at f2_opp50 w/o decreasing VDDx voltage */
+  ret = omap_device_scale(mdev2, dev2, f2_opp50);
+  if (ret)
+  	/* handle error *.
-- 
1.7.0.4

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

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

[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

[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

[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 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 10/13] OMAP3: Add voltage dependency table for VDD1.

[Kevin Hilman]

Hi Vishwa,

Vishwanath BS <vishwanath.bs@ti.com> writes:

> From: Thara Gopinath <thara@ti.com>
>
> 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>

As you are now on the delivery path, your signoff should be here as
well.

Some initial tests on 34xx turned up errors when trying to scale
voltage:

   omap_voltage_get_voltdata: Unable to match the current voltage with  the voltagetable for vdd_core

First, this error message wasn't very useful since I had no idea what
the "current" voltage was.  Also, "current voltage" probably isn't the
right word since to me, current voltage means the one currently set.
This is actually the target voltage that is being looked up.

In any case, root cause below...

> This patch has checkpatch warnings for line over 80 chars. It is not fixed for
> code readability.

OK, this can either be left out, or added below the '---' as it's not
needed in the final git history.

> ---
>  arch/arm/mach-omap2/voltage.c |   42 +++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 40 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/voltage.c b/arch/arm/mach-omap2/voltage.c
> index 92fe20d..8881c0c 100644
> --- a/arch/arm/mach-omap2/voltage.c
> +++ b/arch/arm/mach-omap2/voltage.c
> @@ -191,6 +191,39 @@ static struct omap_volt_data omap44xx_vdd_core_volt_data[] = {
>  	VOLT_DATA_DEFINE(0, 0, 0, 0),
>  };
>  
> +/* OMAP 3430 MPU Core VDD dependency table */
> +static struct omap_vdd_dep_volt omap34xx_vdd1_vdd2_data[] = {
> +	{.main_vdd_volt = OMAP3430_VDD_MPU_OPP1_UV, .dep_vdd_volt = OMAP4430_VDD_CORE_OPP50_UV},
> +	{.main_vdd_volt = OMAP3430_VDD_MPU_OPP2_UV, .dep_vdd_volt = OMAP4430_VDD_CORE_OPP50_UV},
> +	{.main_vdd_volt = OMAP3430_VDD_MPU_OPP3_UV, .dep_vdd_volt = OMAP4430_VDD_CORE_OPP100_UV},
> +	{.main_vdd_volt = OMAP3430_VDD_MPU_OPP4_UV, .dep_vdd_volt = OMAP4430_VDD_CORE_OPP100_UV},
> +	{.main_vdd_volt = OMAP3430_VDD_MPU_OPP5_UV, .dep_vdd_volt = OMAP4430_VDD_CORE_OPP100_UV},
> +	{.main_vdd_volt = 0, .dep_vdd_volt = 0},
> +};

This 34xx table has 4430 OPP voltages for CORE, which are clearly not
right.  34xx has 3 CORE voltages to pick from, so I'm not sure which
ones are right

Please update this with the correct 34xx voltages and also validate on
34xx also.

Thanks,

Kevin


> +static struct omap_vdd_dep_info omap34xx_vdd1_dep_info[] = {
> +	{
> +		.name	= "core",
> +		.dep_table = omap34xx_vdd1_vdd2_data,
> +	},
> +};
> +
> +/* OMAP 3630 MPU Core VDD dependency table */
> +static struct omap_vdd_dep_volt omap36xx_vdd1_vdd2_data[] = {
> +	{.main_vdd_volt = OMAP3630_VDD_MPU_OPP50_UV, .dep_vdd_volt = OMAP3630_VDD_CORE_OPP50_UV},
> +	{.main_vdd_volt = OMAP3630_VDD_MPU_OPP100_UV, .dep_vdd_volt = OMAP3630_VDD_CORE_OPP100_UV},
> +	{.main_vdd_volt = OMAP3630_VDD_MPU_OPP120_UV, .dep_vdd_volt = OMAP3630_VDD_CORE_OPP100_UV},
> +	{.main_vdd_volt = OMAP3630_VDD_MPU_OPP1G_UV, .dep_vdd_volt = OMAP3630_VDD_CORE_OPP100_UV},
> +	{.main_vdd_volt = 0, .dep_vdd_volt = 0},
> +};
> +
> +static struct omap_vdd_dep_info omap36xx_vdd1_dep_info[] = {
> +	{
> +		.name	= "core",
> +		.dep_table = omap36xx_vdd1_vdd2_data,
> +	},
> +};
> +
>  static struct dentry *voltage_dir;
>  
>  /* Init function pointers */
> @@ -696,10 +729,15 @@ static int __init omap3_vdd_data_configure(struct omap_vdd_info *vdd)
>  	}
>  
>  	if (!strcmp(vdd->voltdm.name, "mpu")) {
> -		if (cpu_is_omap3630())
> +		if (cpu_is_omap3630()) {
>  			vdd->volt_data = omap36xx_vddmpu_volt_data;
> -		else
> +			vdd->dep_vdd_info = omap36xx_vdd1_dep_info;
> +			vdd->nr_dep_vdd = ARRAY_SIZE(omap36xx_vdd1_dep_info);
> +		} else {
>  			vdd->volt_data = omap34xx_vddmpu_volt_data;
> +			vdd->dep_vdd_info = omap34xx_vdd1_dep_info;
> +			vdd->nr_dep_vdd = ARRAY_SIZE(omap34xx_vdd1_dep_info);
> +		}
>  
>  		vdd->vp_reg.tranxdone_status = OMAP3430_VP1_TRANXDONE_ST_MASK;
>  		vdd->vc_reg.cmdval_reg = OMAP3_PRM_VC_CMD_VAL_0_OFFSET;

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

[Vishwanath Sripathy]

Kevin,

> -----Original Message-----
> From: Kevin Hilman [mailto:khilman@ti.com]
> Sent: Saturday, January 29, 2011 6:01 AM
> To: Vishwanath BS
> Cc: linux-omap@vger.kernel.org; patches@linaro.org; Thara Gopinath
> Subject: Re: [PATCH 10/13] OMAP3: Add voltage dependency table for
> VDD1.
>
> Hi Vishwa,
>
> Vishwanath BS <vishwanath.bs@ti.com> writes:
>
> > From: Thara Gopinath <thara@ti.com>
> >
> > 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>
>
> As you are now on the delivery path, your signoff should be here as
> well.
OK
>
> Some initial tests on 34xx turned up errors when trying to scale
> voltage:
>
>    omap_voltage_get_voltdata: Unable to match the current voltage with
> the voltagetable for vdd_core
>
> First, this error message wasn't very useful since I had no idea what
> the "current" voltage was.  Also, "current voltage" probably isn't the
> right word since to me, current voltage means the one currently set.
> This is actually the target voltage that is being looked up.

Agreed. Will fix it in the next version.
>
> In any case, root cause below...
>
> > This patch has checkpatch warnings for line over 80 chars. It is not
> fixed for
> > code readability.
>
> OK, this can either be left out, or added below the '---' as it's not
> needed in the final git history.

OK

>
> > ---
> >  arch/arm/mach-omap2/voltage.c |   42
> +++++++++++++++++++++++++++++++++++++++-
> >  1 files changed, 40 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/voltage.c b/arch/arm/mach-
> omap2/voltage.c
> > index 92fe20d..8881c0c 100644
> > --- a/arch/arm/mach-omap2/voltage.c
> > +++ b/arch/arm/mach-omap2/voltage.c
> > @@ -191,6 +191,39 @@ static struct omap_volt_data
> omap44xx_vdd_core_volt_data[] = {
> >  	VOLT_DATA_DEFINE(0, 0, 0, 0),
> >  };
> >
> > +/* OMAP 3430 MPU Core VDD dependency table */
> > +static struct omap_vdd_dep_volt omap34xx_vdd1_vdd2_data[] = {
> > +	{.main_vdd_volt = OMAP3430_VDD_MPU_OPP1_UV,
> .dep_vdd_volt = OMAP4430_VDD_CORE_OPP50_UV},
> > +	{.main_vdd_volt = OMAP3430_VDD_MPU_OPP2_UV,
> .dep_vdd_volt = OMAP4430_VDD_CORE_OPP50_UV},
> > +	{.main_vdd_volt = OMAP3430_VDD_MPU_OPP3_UV,
> .dep_vdd_volt = OMAP4430_VDD_CORE_OPP100_UV},
> > +	{.main_vdd_volt = OMAP3430_VDD_MPU_OPP4_UV,
> .dep_vdd_volt = OMAP4430_VDD_CORE_OPP100_UV},
> > +	{.main_vdd_volt = OMAP3430_VDD_MPU_OPP5_UV,
> .dep_vdd_volt = OMAP4430_VDD_CORE_OPP100_UV},
> > +	{.main_vdd_volt = 0, .dep_vdd_volt = 0},
> > +};
>
> This 34xx table has 4430 OPP voltages for CORE, which are clearly not
> right.  34xx has 3 CORE voltages to pick from, so I'm not sure which
> ones are right
>
> Please update this with the correct 34xx voltages and also validate on
> 34xx also.
Yes, will fix it.
Regarding 34xx testing, I did try to test it on 3430 SDP. However image
was not booting up when I used pm branch.
I see that with latest pm-core branch image boots up where as with pm
branch it does not. Is this a known issue?

Vishwa
>
> Thanks,
>
> Kevin
>
>
> > +static struct omap_vdd_dep_info omap34xx_vdd1_dep_info[] = {
> > +	{
> > +		.name	= "core",
> > +		.dep_table = omap34xx_vdd1_vdd2_data,
> > +	},
> > +};
> > +
> > +/* OMAP 3630 MPU Core VDD dependency table */
> > +static struct omap_vdd_dep_volt omap36xx_vdd1_vdd2_data[] = {
> > +	{.main_vdd_volt = OMAP3630_VDD_MPU_OPP50_UV,
> .dep_vdd_volt = OMAP3630_VDD_CORE_OPP50_UV},
> > +	{.main_vdd_volt = OMAP3630_VDD_MPU_OPP100_UV,
> .dep_vdd_volt = OMAP3630_VDD_CORE_OPP100_UV},
> > +	{.main_vdd_volt = OMAP3630_VDD_MPU_OPP120_UV,
> .dep_vdd_volt = OMAP3630_VDD_CORE_OPP100_UV},
> > +	{.main_vdd_volt = OMAP3630_VDD_MPU_OPP1G_UV,
> .dep_vdd_volt = OMAP3630_VDD_CORE_OPP100_UV},
> > +	{.main_vdd_volt = 0, .dep_vdd_volt = 0},
> > +};
> > +
> > +static struct omap_vdd_dep_info omap36xx_vdd1_dep_info[] = {
> > +	{
> > +		.name	= "core",
> > +		.dep_table = omap36xx_vdd1_vdd2_data,
> > +	},
> > +};
> > +
> >  static struct dentry *voltage_dir;
> >
> >  /* Init function pointers */
> > @@ -696,10 +729,15 @@ static int __init
> omap3_vdd_data_configure(struct omap_vdd_info *vdd)
> >  	}
> >
> >  	if (!strcmp(vdd->voltdm.name, "mpu")) {
> > -		if (cpu_is_omap3630())
> > +		if (cpu_is_omap3630()) {
> >  			vdd->volt_data = omap36xx_vddmpu_volt_data;
> > -		else
> > +			vdd->dep_vdd_info = omap36xx_vdd1_dep_info;
> > +			vdd->nr_dep_vdd =
> ARRAY_SIZE(omap36xx_vdd1_dep_info);
> > +		} else {
> >  			vdd->volt_data = omap34xx_vddmpu_volt_data;
> > +			vdd->dep_vdd_info = omap34xx_vdd1_dep_info;
> > +			vdd->nr_dep_vdd =
> ARRAY_SIZE(omap34xx_vdd1_dep_info);
> > +		}
> >
> >  		vdd->vp_reg.tranxdone_status =
> OMAP3430_VP1_TRANXDONE_ST_MASK;
> >  		vdd->vc_reg.cmdval_reg =
> OMAP3_PRM_VC_CMD_VAL_0_OFFSET;

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

[Kevin Hilman]

Vishwanath Sripathy <vishwanath.bs@ti.com> writes:

[...]

> Regarding 34xx testing, I did try to test it on 3430 SDP. However
> image was not booting up when I used pm branch.  I see that with
> latest pm-core branch image boots up where as with pm branch it does
> not. Is this a known issue?

I don't have a 3430SDP, but it boots for me on 3430/n900, 3530/omap3evm
and 3630/zoom3.

Kevin

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 01/13] OMAP: Introduce accessory APIs for DVFS

[Kevin Hilman]

Vishwanath BS <vishwanath.bs@ti.com> writes:

> This patch introduces accessory APIs for DVFS.

Actually, it begins the implementation of a DVFS layer.

> Data structures added:
> 1. omap_dev_user_list: This structure maintains list of frequency requests per
>    device basis. When a device needs to be scaled to a particular frequency,
>    This list is searched to find the maximum request for a given device.
>    If noone has placed any request, device frequency is obtained from device
>    opp table.
> 2. omap_vdd_dev_list: This strcucture stores device list per vdd basis.
>    Whenever a device is registered with a vdd, it is added to this list.
> 3. omap_vdd_user_list: 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.
> 4. omap_vdd_dvfs_info: This structure is used to abstract DVFS related
>    information per VDD basis. It holds pointer to corresponding vdd's
>    voltagedomain instance and pointer to user list.

The terms "user" and dev/device are rather overloaded here and elsewhere
in the code, and makes for rather difficult reading.

I have an idea (or two) to rework these data structures and how they're
stored/connected, but I need to think on it a little more.  More on that
tomorrow...

> Following APIs have been added to operate on above data structures:
> 1. omap_dvfs_add_vdd_user - API to add a user into omap_vdd_user_list
> 2. omap_vdd_user_list - API to remove a user from omap_vdd_user_list

huh?  cut and paste error?  I think this was supposed to be
_remove_vdd_user() ?

> 3. omap_dvfs_register_device - API to register a device with vdd
> 4. omap_dvfs_add_freq_request - API to add a frequency request into
>    omap_dev_user_list
> 5. omap_dvfs_remove_freq_request - API to remove a frequency request from
>    omap_dev_user_list
> 6. omap_dvfs_find_voltage - API to find the opp corresponding to given voltage

I think function naming needs rework for consistency.  For example, to
request a frequency you call _add_freq_request(), but to add a voltage
you call _add_vdd_user(), not very reader friendly

How about simply:

    omap_dvfs_request_freq()
    omap_dvfs_request_volt()

with remove equivalents.

Actually, looking more at the functions in this patch, the frequency and
voltage functions here are basically identical.  There really isn't any
good reason to have separate functions for frequency and voltage.  How
about:

    omap_dvfs_add_request(dvfs_info, class, req_dev, target_dev);
    omap_dvfs_remove_request(dvfs_info, class, req_dev, target_dev);

where class = OMAP_DVFS_FREQ | OMAP_DVFS_VOLT.  This way, based on the
class, you can add the requests to the separate lists, but bulk
of the function is the same.

>
> DVFS layer is initialized and basic data structures are allocated and
> initialized as part of this.
>
> This patch is based on Thara's previous DVFS implementation, but with major
> rework.
>

Minor comment on acronyms.   In the comments throughout, please
capitalize acronyms like DVFS, VDD, etc.  Currently, it is mixed between
upper and lower-case.

> Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
> Cc: Thara Gopinath <thara@ti.com>
> ---
>  arch/arm/mach-omap2/Makefile           |    2 +-
>  arch/arm/mach-omap2/dvfs.c             |  456 ++++++++++++++++++++++++++++++++
>  arch/arm/plat-omap/include/plat/dvfs.h |   27 ++
>  arch/arm/plat-omap/omap_device.c       |    9 +
>  4 files changed, 493 insertions(+), 1 deletions(-)
>  create mode 100644 arch/arm/mach-omap2/dvfs.c
>  create mode 100644 arch/arm/plat-omap/include/plat/dvfs.h
>
> diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
> index 4ab82f6..bb2e2bc 100644
> --- a/arch/arm/mach-omap2/Makefile
> +++ b/arch/arm/mach-omap2/Makefile
> @@ -61,7 +61,7 @@ ifeq ($(CONFIG_PM),y)
>  obj-$(CONFIG_ARCH_OMAP2)		+= pm24xx.o
>  obj-$(CONFIG_ARCH_OMAP2)		+= sleep24xx.o pm_bus.o voltage.o
>  obj-$(CONFIG_ARCH_OMAP3)		+= pm34xx.o sleep34xx.o voltage.o \
> -					   cpuidle34xx.o pm_bus.o
> +					   cpuidle34xx.o pm_bus.o dvfs.o
>  obj-$(CONFIG_ARCH_OMAP4)		+= pm44xx.o voltage.o pm_bus.o
>  obj-$(CONFIG_PM_DEBUG)			+= pm-debug.o
>  obj-$(CONFIG_OMAP_SMARTREFLEX)          += sr_device.o smartreflex.o
> diff --git a/arch/arm/mach-omap2/dvfs.c b/arch/arm/mach-omap2/dvfs.c
> new file mode 100644
> index 0000000..8832e4a
> --- /dev/null
> +++ b/arch/arm/mach-omap2/dvfs.c
> @@ -0,0 +1,456 @@
> +/*
> + * OMAP3/OMAP4 DVFS Management Routines
> + *
> + * Author: Vishwanath BS	<vishwanath.bs@ti.com>
> + *
> + * Copyright (C) 2011 Texas Instruments, Inc.
> + * Vishwanath BS <vishwanath.bs@ti.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/spinlock.h>
> +#include <linux/plist.h>
> +#include <linux/slab.h>
> +#include <linux/opp.h>
> +#include <plat/common.h>
> +#include <plat/voltage.h>
> +#include <plat/omap_device.h>
> +
> +/**
> + * struct omap_dev_user_list - Structure maitain userlist per devide

typos: maintain, device

the latter typo you fix in a follow up patch, but should be fixed here.

> + *
> + * @dev:       The device requesting for a particular frequency
> + * @node:      The list head entry
> + * @freq:      frequency being requested
> + *
> + * Using this structure, user list (requesting dev * and frequency) for
> + * each device is maintained. This is how we can have different devices
> + * at different frequencies (to support frequency locking and throttling).
> + * Even if one of the devices in a given vdd has locked it's frequency,
> + * other's can still scale their frequency using this list.
> + * If no one has placed a frequency request for a device, then device is
> + * set to the frequency from it's opp table.
> + */
> +struct omap_dev_user_list {
> +		struct device *dev;
> +		struct plist_node node;
> +		u32 freq;

extra indentation

> +};
> +
> +/**
> + * struct omap_vdd_dev_list - Device list per vdd
> + *
> + * @dev:	The device belonging to a particular vdd
> + * @node:	The list head entry

and the other entries?

> + */
> +struct omap_vdd_dev_list {
> +	struct device *dev;
> +	struct list_head node;
> +	struct plist_head user_list;
> +	spinlock_t user_lock; /* spinlock for plist */

comment redundant

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

This struct is identical to omap_dev_user_list above, except
s/freq/volt/.  You probably just need a single struct using 'val'
instead of freq/volt.

That being said, why is the frequency struct called 'dev_user_list' and
the voltage table called 'vdd_user_list'.  This alone renders the
resulting code unreadable.  


> +/**
> + * struct omap_vdd_dvfs_info - The per vdd dvfs info
> + *
> + * @user_lock:	spinlock for plist operations
> + * @user_list:	The vdd user list
> + * @scaling_mutex:	Mutex for protecting dvfs data structures for a vdd
> + * @voltdm: Voltage domains for which dvfs info stored

missing some entries here

> + * This is a fundamental structure used to store all the required
> + * DVFS related information for a vdd.
> + */
> +struct omap_vdd_dvfs_info {
> +	spinlock_t user_lock; /* spin lock */

comment redundant

> +	struct plist_head user_list;
> +	struct mutex scaling_mutex; /* dvfs mutex */

ditto

> +	struct voltagedomain *voltdm;
> +	struct list_head dev_list;
> +};
> +
> +static struct omap_vdd_dvfs_info *omap_dvfs_info_list;
> +static int omap_nr_vdd;
> +
> +static struct voltagedomain omap3_vdd[] = {
> +	{
> +	.name = "mpu",
> +	},
> +	{
> +	.name = "core",
> +	},
> +};

indentation

> +static int __init omap_dvfs_init(void);
> +
> +static struct omap_vdd_dvfs_info *get_dvfs_info(struct voltagedomain *voltdm)
> +{
> +	int i;

insert blank line

> +	if (!voltdm || !omap_dvfs_info_list)
> +		return NULL;
> +
> +	for (i = 0; i < omap_nr_vdd; i++)
> +		if (omap_dvfs_info_list[i].voltdm == voltdm)
> +			return &omap_dvfs_info_list[i];
> +
> +	pr_warning("%s: unable find dvfs info for vdd %s\n",
> +			__func__, voltdm->name);
> +	return NULL;
> +}
> +
> +/**
> + * omap_dvfs_find_voltage() - search for given 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

comment doesn't match code.  OPP lookup is done using the 'ceil'
version, which is not an exact match, and the voltage check uses '>='
not '=='.

> + * opp if found, else returns ERR_PTR in case of error and should be handled
> + * using IS_ERR. If there are multiple opps with same voltage, it will return
> + * the first available entry.

More specifically, it will return the one with the lowest frequency.

> + */
> +static struct opp *omap_dvfs_find_voltage(struct device *dev,
> +		unsigned long volt)
> +{
> +	struct opp *opp = ERR_PTR(-ENODEV);
> +	unsigned long f = 0;
> +
> +	do {
> +		opp = opp_find_freq_ceil(dev, &f);
> +		if (IS_ERR(opp))
> +			break;
> +		if (opp_get_voltage(opp) >= volt)
> +			break;
> +		f++;
> +	} while (1);
> +
> +	return opp;
> +}
> +
> +/**
> + * omap_dvfs_add_vdd_user() - Add a voltage request
> + * @dvfs_info: omap_vdd_dvfs_info pointer for the required vdd
> + * @dev: device making the request
> + * @volt: requesting voltage in uV
> + *
> + * Adds the given devices' voltage request into corresponding
> + * vdd's omap_vdd_dvfs_info user list (plist). This list is used
> + * to find the maximum voltage request for a given vdd.
> + *
> + * Returns 0 on success.
> + */
> +static int omap_dvfs_add_vdd_user(struct omap_vdd_dvfs_info *dvfs_info,
> +		struct device *dev, unsigned long volt)
> +{
> +	struct omap_vdd_user_list *user = NULL, *temp_user;
> +	struct plist_node *node;
> +
> +	if (!dvfs_info || IS_ERR(dvfs_info)) {
> +		dev_warn(dev, "%s: VDD specified does not exist!\n", __func__);
> +		return -EINVAL;
> +	}
> +
> +	mutex_lock(&dvfs_info->scaling_mutex);
> +
> +	plist_for_each_entry(temp_user, &dvfs_info->user_list, node) {
> +		if (temp_user->dev == dev) {
> +			user = temp_user;
> +			break;
> +		}
> +	}
> +
> +	if (!user) {
> +		user = kzalloc(sizeof(struct omap_vdd_user_list), GFP_KERNEL);
> +		if (!user) {
> +			dev_err(dev, "%s: Unable to creat a new user for vdd_%s\n",
> +				__func__, dvfs_info->voltdm->name);
> +			mutex_unlock(&dvfs_info->scaling_mutex);
> +			return -ENOMEM;
> +		}
> +		user->dev = dev;
> +	} else {
> +		plist_del(&user->node, &dvfs_info->user_list);
> +	}
> +
> +	plist_node_init(&user->node, volt);
> +	plist_add(&user->node, &dvfs_info->user_list);
> +	node = plist_last(&dvfs_info->user_list);
> +	user->volt = node->prio;
> +
> +	mutex_unlock(&dvfs_info->scaling_mutex);
> +
> +	return 0;
> +}
> +
> +/**
> + * omap_dvfs_remove_vdd_user() - Remove a voltage request
> + * @dvfs_info: omap_vdd_dvfs_info pointer for the required vdd
> + * @dev: device making the request
> + *
> + * Removes the given devices' voltage request from corresponding
> + * vdd's omap_vdd_dvfs_info user list (plist).
> + *
> + * Returns 0 on success.
> + */
> +static int omap_dvfs_remove_vdd_user(struct omap_vdd_dvfs_info *dvfs_info,
> +		struct device *dev)
> +{
> +	struct omap_vdd_user_list *user = NULL, *temp_user;
> +	int ret = 0;
> +
> +	if (!dvfs_info || IS_ERR(dvfs_info)) {
> +		dev_err(dev, "%s: VDD specified does not exist!\n", __func__);
> +		return -EINVAL;
> +	}
> +
> +	mutex_lock(&dvfs_info->scaling_mutex);
> +
> +	plist_for_each_entry(temp_user, &dvfs_info->user_list, node) {
> +		if (temp_user->dev == dev) {
> +			user = temp_user;
> +			break;
> +		}
> +	}
> +
> +	if (user)
> +		plist_del(&user->node, &dvfs_info->user_list);
> +	else {
> +		dev_err(dev, "%s: Unable to find the user for vdd_%s\n",
> +					__func__, dvfs_info->voltdm->name);
> +		ret = -ENOMEM;
> +	}
> +	mutex_unlock(&dvfs_info->scaling_mutex);
> +
> +	return ret;
> +}
> +
> +/**
> + * omap_dvfs_register_device - Add a device into voltage domain
> + * @voltdm:	voltage domain to which the device is to be added
> + * @dev:	Device to be added
> + *
> + * This API will add a given device into user_list of corresponding
> + * vdd's omap_vdd_dvfs_info strucure. This list is traversed to scale
> + * frequencies of all the devices on a given vdd. This api is called
> + * while hwmod db is built for an omap_device.
> + *
> + * Returns 0 on success.
> + */
> +int omap_dvfs_register_device(struct voltagedomain *voltdm, struct device *dev)
> +{
> +	struct omap_vdd_dev_list *temp_dev;

The point of using the 'temp_' names is for temporary iterators.  Here,
you're using it as the dev_list throughout.  Just call it dev_list.

> +	struct omap_vdd_dvfs_info *dvfs_info = get_dvfs_info(voltdm);
> +
> +	if (!voltdm || IS_ERR(voltdm) || !dvfs_info) {
> +		dev_warn(dev, "%s: VDD specified does not exist!\n", __func__);
> +		return -EINVAL;
> +	}
> +
> +	list_for_each_entry(temp_dev, &dvfs_info->dev_list, node) {
> +		if (temp_dev->dev == dev) {
> +			dev_warn(dev, "%s: Device already added to vdee_%s\n",

s/vdee/VDD/ ?

You might be seeing device already added for devices with multiple
hwmods per omap_device.  More on this below.

> +				__func__, dvfs_info->voltdm->name);
> +			return -EINVAL;
> +		}
> +	}
> +
> +	temp_dev = kzalloc(sizeof(struct omap_vdd_dev_list), GFP_KERNEL);
> +	if (!temp_dev) {
> +		dev_err(dev, "%s: Unable to creat a new device for vdd_%s\n",
> +			__func__, dvfs_info->voltdm->name);
> +		return -ENOMEM;
> +	}
> +
> +	/* Initialize priority ordered list */
> +	spin_lock_init(&temp_dev->user_lock);
> +	plist_head_init(&temp_dev->user_list, &temp_dev->user_lock);
> +
> +	temp_dev->dev = dev;
> +	list_add(&temp_dev->node, &dvfs_info->dev_list);
> +
> +	return 0;
> +}
> +
> +/**
> + * omap_dvfs_add_freq_request() - add a requested device frequency
> + *
> + *
> + * @dvfs_info: omap_vdd_dvfs_info pointer for the required vdd
> + * @req_dev: device making the request
> + * @target_dev: target device for which frequency request is being made
> + * @freq:	target device frequency
> + *
> + * This API adds a requested frequency into target's device frequency list.

more documentation needed for the reasoning behind why both a requesting
device and a target device are needed.  Also, whey they are needed for
frequency and not voltage.  That might affect my idea above about having
the same function for adding freq/voltage requests.  Even so, the core
part of these functions is identical, so some common functions should
probably be used.

> + * Returns 0 on success.
> + */
> +static int omap_dvfs_add_freq_request(struct omap_vdd_dvfs_info *dvfs_info,
> +	struct device *req_dev, struct device *target_dev, unsigned long freq)
> +{
> +	struct omap_dev_user_list *dev_user = NULL, *tmp_user;
> +	struct omap_vdd_dev_list *temp_dev;

	struct omap_vdd_dev_list *dev_list;

> +	if (!dvfs_info || IS_ERR(dvfs_info)) {
> +		dev_warn(target_dev, "%s: VDD specified does not exist!\n",
> +			__func__);
> +		return -EINVAL;
> +	}
> +
> +	mutex_lock(&dvfs_info->scaling_mutex);
> +
> +	list_for_each_entry(temp_dev, &dvfs_info->dev_list, node) {
> +		if (temp_dev->dev == target_dev)

			dev_list = temp_dev;

> +			break;
> +	}
> +	if (temp_dev->dev != target_dev) {

	if (!dev_list) {

> +		dev_warn(target_dev, "%s: target_dev does not exist!\n",
> +			__func__);
> +		mutex_unlock(&dvfs_info->scaling_mutex);
> +		return -EINVAL;
> +	}
> +
> +	plist_for_each_entry(tmp_user, &temp_dev->user_list, node) {
> +		if (tmp_user->dev == req_dev) {
> +			dev_user = tmp_user;
> +			break;
> +		}
> +	}
> +
> +	if (!dev_user) {
> +		dev_user = kzalloc(sizeof(struct omap_dev_user_list),
> +					GFP_KERNEL);
> +		if (!dev_user) {
> +			dev_err(target_dev, "%s: Unable to creat a new user for vdd_%s\n",
> +				__func__, dvfs_info->voltdm->name);
> +			mutex_unlock(&dvfs_info->scaling_mutex);
> +			return -ENOMEM;
> +		}
> +		dev_user->dev = req_dev;
> +	} else {
> +		plist_del(&dev_user->node, &temp_dev->user_list);
> +	}
> +
> +	plist_node_init(&dev_user->node, freq);
> +	plist_add(&dev_user->node, &temp_dev->user_list);
> +
> +	mutex_unlock(&dvfs_info->scaling_mutex);
> +	return 0;
> +}
> +
> +/**
> + * omap_dvfs_remove_freq_request() - Remove the requested device frequency
> + *
> + * @dvfs_info: omap_vdd_dvfs_info pointer for the required vdd
> + * @req_dev: device removing the request
> + * @target_dev: target device from which frequency request is being removed
> + *
> + * This API removes a requested frequency from target's device frequency list.
> + *
> + * Returns 0 on success.
> + */
> +
> +static int omap_dvfs_remove_freq_request(struct omap_vdd_dvfs_info *dvfs_info,
> +	struct device *req_dev, struct device *target_dev)
> +{
> +	struct omap_dev_user_list *dev_user = NULL, *tmp_user;
> +	int ret = 0;
> +	struct omap_vdd_dev_list *temp_dev;
> +
> +	if (!dvfs_info || IS_ERR(dvfs_info)) {
> +		dev_warn(target_dev, "%s: VDD specified does not exist!\n",
> +			__func__);
> +		return -EINVAL;
> +	}
> +
> +	mutex_lock(&dvfs_info->scaling_mutex);
> +
> +	list_for_each_entry(temp_dev, &dvfs_info->dev_list, node) {
> +		if (temp_dev->dev == target_dev)
> +			break;
> +	}
> +
> +	if (temp_dev->dev != target_dev) {
> +		dev_warn(target_dev, "%s: target_dev does not exist!\n",
> +			__func__);
> +		mutex_unlock(&dvfs_info->scaling_mutex);
> +		return -EINVAL;
> +	}
> +
> +	plist_for_each_entry(tmp_user, &temp_dev->user_list, node) {
> +		if (tmp_user->dev == req_dev) {
> +			dev_user = tmp_user;
> +			break;
> +		}
> +	}
> +
> +	if (dev_user)
> +		plist_del(&dev_user->node, &temp_dev->user_list);
> +	else {
> +		dev_err(target_dev, "%s: Unable to remove the user for vdd_%s\n",
> +				__func__, dvfs_info->voltdm->name);
> +			ret = -EINVAL;
> +		}
> +
> +	return ret;
> +}
> +
> +/**
> + * omap_dvfs_init() - Initialize omap dvfs layer
> + *
> + * Initalizes omap dvfs layer. It basically allocates memory for
> + * omap_dvfs_info_list and  populates voltdm pointer inside
> + * omap_vdd_dvfs_info structure for all the VDDs.
> + *
> + * Returns 0 on success.
> + */
> +static int __init omap_dvfs_init()
> +{
> +	int i;
> +	struct voltagedomain *vdd_list;

insert blank line

> +	if (cpu_is_omap34xx()) {
> +		omap_nr_vdd = 2;

use ARRAY_SIZE(omap3_vdd)

> +		vdd_list = omap3_vdd;
> +	}

else? 

> +	omap_dvfs_info_list = kzalloc(omap_nr_vdd *
> +			sizeof(struct omap_vdd_dvfs_info), GFP_KERNEL);
> +	if (!omap_dvfs_info_list) {
> +		pr_warning("%s: Unable to allocate memory for vdd_list",
> +			__func__);
> +		return -ENOMEM;
> +	}
> +
> +	for (i = 0; i < omap_nr_vdd; i++) {
> +		omap_dvfs_info_list[i].voltdm =
> +			omap_voltage_domain_lookup(vdd_list[i].name);
> +		/* Init the plist */

comments redundant

> +		spin_lock_init(&omap_dvfs_info_list[i].user_lock);
> +		plist_head_init(&omap_dvfs_info_list[i].user_list,
> +					&omap_dvfs_info_list[i].user_lock);

ditto

> +		/* Init the DVFS mutex */
> +		mutex_init(&omap_dvfs_info_list[i].scaling_mutex);
> +		/* Init the device list */
 
ditto

> +		INIT_LIST_HEAD(&omap_dvfs_info_list[i].dev_list);


> +	}
> +
> +	return 0;
> +}
> +core_initcall(omap_dvfs_init);
> diff --git a/arch/arm/plat-omap/include/plat/dvfs.h b/arch/arm/plat-omap/include/plat/dvfs.h
> new file mode 100644
> index 0000000..1302990
> --- /dev/null
> +++ b/arch/arm/plat-omap/include/plat/dvfs.h
> @@ -0,0 +1,27 @@
> +/*
> + * OMAP3/OMAP4 DVFS Management Routines
> + *
> + * Author: Vishwanath BS	<vishwanath.bs@ti.com>
> + *
> + * Copyright (C) 2011 Texas Instruments, Inc.
> + * Vishwanath BS <vishwanath.bs@ti.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef __ARCH_ARM_MACH_OMAP2_DVFS_H
> +#define __ARCH_ARM_MACH_OMAP2_DVFS_H
> +#include <plat/voltage.h>
> +
> +#ifdef CONFIG_PM
> +int omap_dvfs_register_device(struct voltagedomain *voltdm, struct device *dev);
> +#else
> +static inline int omap_dvfs_register_device(struct voltagedomain *voltdm,
> +		struct device *dev)
> +{
> +	return -EINVAL;
> +}
> +#endif
> +#endif
> diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
> index 57adb27..a84e849 100644
> --- a/arch/arm/plat-omap/omap_device.c
> +++ b/arch/arm/plat-omap/omap_device.c
> @@ -86,6 +86,7 @@
>  
>  #include <plat/omap_device.h>
>  #include <plat/omap_hwmod.h>
> +#include <plat/dvfs.h>
>  
>  /* These parameters are passed to _omap_device_{de,}activate() */
>  #define USE_WAKEUP_LAT			0
> @@ -481,6 +482,14 @@ struct omap_device *omap_device_build_ss(const char *pdev_name, int pdev_id,
>  	for (i = 0; i < oh_cnt; i++) {
>  		hwmods[i]->od = od;
>  		_add_optional_clock_alias(od, hwmods[i]);
> +		if (!is_early_device && hwmods[i]->vdd_name) {
> +			struct omap_hwmod *oh = hwmods[i];
> +			struct voltagedomain *voltdm;
> +
> +			voltdm = omap_voltage_domain_lookup(oh->vdd_name);
> +			if (!omap_dvfs_register_device(voltdm, &od->pdev.dev))
> +				oh->voltdm = voltdm;
> +		}

This should be taken out of the for loop.  Otherwise, if an omap_device
has multiple hwmods, then the same devices is registered with DVFS
twice.

>  	}
>  
>  	if (ret)

Kevin

Re: [PATCH 02/13] OMAP: Introduce device specific set rate and get rate in omap_device structure

[Kevin Hilman]

Vishwanath BS <vishwanath.bs@ti.com> writes:

> From: Thara Gopinath <thara@ti.com>
>
> This patch extends the omap_device 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 omap device layer
> namely omap_device_set_rate that can be called to set a new operating
> rate for a device, omap_device_get_rate that can be called to retrieve
> the operating frequency for a device and omap_device_register_dvfs_callbacks
> to register the device specific set_rate and get_rate functions.
> The omap_device_set_rate and omap_device_get_rate does some routine error
> checks and finally calls into the device specific set_rate
> and get_rate APIs populated through omap_device_populate_rate_fns.
>
> Signed-off-by: Thara Gopinath <thara@ti.com>
> Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
> ---
>  arch/arm/plat-omap/include/plat/omap_device.h |    9 +++++
>  arch/arm/plat-omap/omap_device.c              |   49 +++++++++++++++++++++++++
>  2 files changed, 58 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 e4c349f..204fb0a 100644
> --- a/arch/arm/plat-omap/include/plat/omap_device.h
> +++ b/arch/arm/plat-omap/include/plat/omap_device.h
> @@ -50,6 +50,8 @@ extern struct device omap_device_parent;
>   * @hwmods: (one .. many per omap_device)
>   * @hwmods_cnt: ARRAY_SIZE() of @hwmods
>   * @pm_lats: ptr to an omap_device_pm_latency table
> + * @set_rate: fn ptr to change the operating rate.
> + * @get_rate: fn ptr to retrieve the current operating rate.
>   * @pm_lats_cnt: ARRAY_SIZE() of what is passed to @pm_lats
>   * @pm_lat_level: array index of the last odpl entry executed - -1 if never
>   * @dev_wakeup_lat: dev wakeup latency in nanoseconds
> @@ -73,6 +75,8 @@ struct omap_device {
>  	s8				pm_lat_level;
>  	u8				hwmods_cnt;
>  	u8				_state;
> +	int (*set_rate)(struct device *dev, unsigned long rate);
> +	unsigned long (*get_rate) (struct device *dev);
>  };
>  
>  /* Device driver interface (call via platform_data fn ptrs) */
> @@ -107,6 +111,11 @@ void __iomem *omap_device_get_rt_va(struct omap_device *od);
>  int omap_device_align_pm_lat(struct platform_device *pdev,
>  			     u32 new_wakeup_lat_limit);
>  struct powerdomain *omap_device_get_pwrdm(struct omap_device *od);
> +int omap_device_set_rate(struct device *dev, unsigned long freq);
> +unsigned long omap_device_get_rate(struct device *dev);
> +void omap_device_register_dvfs_callbacks(struct device *dev,
> +		int (*set_rate)(struct device *dev, unsigned long rate),
> +		unsigned long (*get_rate) (struct device *dev));
>  u32 omap_device_get_context_loss_count(struct platform_device *pdev);
>  
>  /* Other */
> diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
> index a84e849..4cee430 100644
> --- a/arch/arm/plat-omap/omap_device.c
> +++ b/arch/arm/plat-omap/omap_device.c
> @@ -810,6 +810,55 @@ int omap_device_enable_clocks(struct omap_device *od)
>  	return 0;
>  }
>  
> +int omap_device_set_rate(struct device *dev, unsigned long freq)
> +{
> +	struct platform_device *pdev;
> +	struct omap_device *od;
> +
> +	pdev = container_of(dev, struct platform_device, dev);

There is a macro for this in device.h:

      pdev = to_platform_device(dev)

This needs to be fixed all over this series.

> +	od = _find_by_pdev(pdev);
> +
> +	if (!od->set_rate) {
> +		dev_err(dev, "%s: No set_rate API for scaling device\n",
> +			__func__);
> +		return -ENODATA;
> +	}
> +
> +	return od->set_rate(dev, freq);
> +}
> +
> +unsigned long omap_device_get_rate(struct device *dev)
> +{
> +	struct platform_device *pdev;
> +	struct omap_device *od;
> +
> +	pdev = container_of(dev, struct platform_device, dev);
> +	od = _find_by_pdev(pdev);

Should also check here for a valid omap_device, but making sure its
parent is omap_device_parent.

> +
> +
> +	if (!od->get_rate) {
> +		dev_err(dev, "%s: No get rate API for the device\n",
> +			__func__);
> +		return 0;
> +	}
> +
> +	return od->get_rate(dev);
> +}
> +
> +void omap_device_register_dvfs_callbacks(struct device *dev,
> +		int (*set_rate)(struct device *dev, unsigned long rate),
> +		unsigned long (*get_rate) (struct device *dev))
> +{
> +	struct platform_device *pdev;
> +	struct omap_device *od;
> +
> +	pdev = container_of(dev, struct platform_device, dev);
> +	od = _find_by_pdev(pdev);
> +

and here

> +	od->set_rate = set_rate;
> +	od->get_rate = get_rate;
> +}
> +
>  struct device omap_device_parent = {
>  	.init_name	= "omap",
>  	.parent         = &platform_bus,

Kevin

Re: [PATCH 03/13] OMAP: Implement Basic DVFS

[Kevin Hilman]

Vishwanath BS <vishwanath.bs@ti.com> writes:

> This patch introduces an API to perform DVFS for a given voltage domain.
> It takes omap_vdd_dvfs_info pointer as input parameter, computes the highest
> requested voltage for that vdd and scales all the devices in that vdd to the
> requested frequency along with voltage scaling.
>
> Based on original patch from Thara.
>
> Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
> Cc: Thara Gopinath <thara@ti.com>
> ---
>  arch/arm/mach-omap2/dvfs.c |   87 +++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 86 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/dvfs.c b/arch/arm/mach-omap2/dvfs.c
> index 8832e4a..cefc2be 100755
> --- a/arch/arm/mach-omap2/dvfs.c
> +++ b/arch/arm/mach-omap2/dvfs.c
> @@ -21,7 +21,7 @@
>  #include <plat/omap_device.h>
>  
>  /**
> - * struct omap_dev_user_list - Structure maitain userlist per devide
> + * struct omap_dev_user_list - Structure maitain userlist per device

this typo should be done in the original patch, not here.

>   *
>   * @dev:       The device requesting for a particular frequency
>   * @node:      The list head entry
> @@ -413,6 +413,91 @@ static int omap_dvfs_remove_freq_request(struct omap_vdd_dvfs_info *dvfs_info,
>  }
>  
>  /**
> + * omap_dvfs_voltage_scale() : API to scale the devices associated with a
> + *						voltage domain vdd voltage.

This function scales both voltage and frequency, so the name
voltage_scale() is a bit misleading.

> + * @dvfs_info: omap_vdd_dvfs_info pointer for the required vdd
> + *
> + * This API runs through the list of devices associated with the
> + * voltage domain and scales the device rates to the one requested
> + * by the user or those corresponding to the new voltage of the
> + * voltage domain. Target voltage is the highest voltage in the vdd_user_list.
> + *
> + * Returns 0 on success
> + * else the error value.
> + */
> +static int omap_dvfs_voltage_scale(struct omap_vdd_dvfs_info *dvfs_info)
> +{
> +	unsigned long curr_volt;
> +	int is_volt_scaled = 0;

should be a bool

> +	struct omap_vdd_dev_list *temp_dev;
> +	struct plist_node *node;
> +	int ret = 0;
> +	struct voltagedomain *voltdm;
> +	unsigned long volt;
> +
> +	if (!dvfs_info || IS_ERR(dvfs_info)) {
> +		pr_warning("%s: VDD specified does not exist!\n", __func__);
> +		return -EINVAL;
> +	}
> +
> +	voltdm = dvfs_info->voltdm;
> +
> +	mutex_lock(&dvfs_info->scaling_mutex);
> +
> +	/* Find the highest voltage being requested */
> +	node = plist_last(&dvfs_info->user_list);
> +	volt = node->prio;
> +
> +	curr_volt = omap_voltage_get_nom_volt(voltdm);
> +
> +	if (curr_volt == volt) {
> +		is_volt_scaled = 1;
> +	} else if (curr_volt < volt) {
> +		ret = omap_voltage_scale_vdd(voltdm, volt);
> +		if (ret) {
> +			pr_warning("%s: Unable to scale the %s to %ld volt\n",
> +						__func__, voltdm->name, volt);
> +			mutex_unlock(&dvfs_info->scaling_mutex);
> +			return ret;
> +		}
> +		is_volt_scaled = 1;
> +	}
> +
> +	list_for_each_entry(temp_dev, &dvfs_info->dev_list, node) {
> +		struct device *dev;
> +		struct opp *opp;
> +		unsigned long freq;
> +
> +		dev = temp_dev->dev;

if you're doing this assignment here, might as well make 'dev' the
iterator instead of temp_dev.

This section would benefit with some comments.  If I understand the code
correctly, something like:

If a frequency has been requested, use the highest requested frequency.

> +		if (!plist_head_empty(&temp_dev->user_list)) {
> +			node = plist_last(&temp_dev->user_list);
> +			freq = node->prio;

otherwise check if device has OPP for this voltage

> +		} else {
> +			opp = omap_dvfs_find_voltage(dev, volt);
> +			if (IS_ERR(opp))
> +				continue;

This needs a comment to, but I'm not sure I understand what's going on
here.  What it seems like:

if this device has no OPP for this voltage, just silently move on to the
next device?   doesn't seem quite right, but not sure I fully grok the
failure modes of omap_dvfs_find_voltage()

Kevin

> +			freq = opp_get_freq(opp);
> +		}
> +
> +		if (freq == omap_device_get_rate(dev)) {
> +			dev_dbg(dev, "%s: Already at the requested"
> +				"rate %ld\n", __func__, freq);
> +			continue;
> +		}
> +
> +		ret |= omap_device_set_rate(dev, freq);
> +	}
> +
> +	if (!is_volt_scaled && !ret)
> +		omap_voltage_scale_vdd(voltdm, volt);
> +
> +	mutex_unlock(&dvfs_info->scaling_mutex);
> +
> +	return 0;
> +}
> +
> +/**
>   * omap_dvfs_init() - Initialize omap dvfs layer
>   *
>   * Initalizes omap dvfs layer. It basically allocates memory for

Re: [PATCH 13/13] OMAP: Add DVFS Documentation

[Kevin Hilman]

Vishwanath BS <vishwanath.bs@ti.com> writes:

> Add Documentation for DVFS Framework
>
> Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
> ---
>  Documentation/arm/OMAP/omap_dvfs |  111 ++++++++++++++++++++++++++++++++++++++
>  1 files changed, 111 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/arm/OMAP/omap_dvfs
>
> diff --git a/Documentation/arm/OMAP/omap_dvfs b/Documentation/arm/OMAP/omap_dvfs
> new file mode 100644
> index 0000000..b026643
> --- /dev/null
> +++ b/Documentation/arm/OMAP/omap_dvfs
> @@ -0,0 +1,111 @@
> +*=============*
> +* DVFS Framework *
> +*=============*
> +(C) 2011 Vishwnath BS <vishwanath.bs@ti.com>, Texas Instruments Incorporated
> +Contents
> +--------
> +1. Introduction
> +2. Data Structure Organization
> +3. DVFS APIs
> +
> +1. Introduction
> +===============
> +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.

Defining the acronym would be helpful here first.

Also, DVFS is not necessarily a "technique".  It also doesn't necessarly
use "optimal" values since the framework would certainly allow you set
optimal settings or sub-optimal settings if you wished.

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

-ENOPARSE

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

OPP is an acronym, please capitalize throughout.

> +   scaled. This is easy now due to the existance of hwmod layer which
> +   allow storing of device specific info. 

Except we don't use hwmod to store OPP data.    Not sure of the point here.

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

What device OPP tables are you referring to here?  The device OPP tables
in opp*_data.c contain neither voltage domain pointers or get_rate
set_rate APIs.

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

This isn't done using use-counting, but rather by keeping lists of
request.

> +3. Keep track of all scalable devices belonging to a particular voltage
> +   domain the voltage layer.

This isn't being done in the voltage layer.

> +4. Keep track of frequency requests for each of the device. This will enable
> +   to scale individual devices to different frequency (even w/o scaling voltage
> +   aka frequency throttling)

> +5. Generic dvfs API that can be called by anybody to scale a device opp.

DVFS is an acronym, please capitalize throughout.

> +   This API takes the device pointer and frequency to which the device
> +   needs to be scaled to. This API then internally finds 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 he device opp table. Then this API will add requested frequency into
> +   the corresponding target device frequency list and add voltage request to
> +   the corresponding vdd. Subsequently it calls voltage scale function which
> +   will find out the highest requested voltage for the given vdd and scales
> +   the voltage to the required one. It also runs 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.

Again, the set_rate pointer is not part of the device OPP table.

> +6. Handle inter VDD dependecies.
> +
> +
> +2.  The Core DVFS data structure:
> +=================================
> +
> + 					|-------------------|			|-------------------|
> + 					|User2 (dev2, freq2)|			|User4 (dev4, freq4)|
> + 					|-------^-----------|			|-------^-----------|
> + 							|								|
> + 					|-------|-----------|			|-------|-----------|
> + 					|User1 (dev1, freq1)|			|User3 (dev3, freq3)|(omap_dev_user_list)
> + 					|-------^-----------|			|-------^-----------|
> +                            |                               |
> + 						|---|--------------|             |------------------|
> + 		     |--------->| DEV1 (dev)       |------------>| DEV2 (dev)       |(omap_vdd_dev_list)
> + 			 |			|omap_dev_user_list|			 |omap_dev_user_list|
> + 			 |			|------------------|             |------------------|
> + 			 |
> +   |---------|-----------|
> +   |       VDD_n         |
> +   |  omap_vdd_dev_list  |
> +   |  omap_vdd_user_list |(omap_vdd_dvfs_info)
> +   |                     |
> +   |--------|------------|
> + 			|
> +            |           |------------|  |------------|  |--------------|
> +            |---------> | vdd_user1  |->|  vdd_user2 |->|   vdd_user3  | (omap_vdd_user_list)
> + 						| (dev, volt)|	| (dev, volt)|  | (dev, volt)  |
> + 						|------------|	|------------|	|--------------|

This diagram is not readable.

> +3.  APIs:
> + =====
> + 1. omap_device_scale - Set a new rate at which the device is to operate
> +
> +  Examples:
> +  1. Simple Device scaling:
> +  Suppose module M wants to put device dev1 to frequency f1. Let's say that mdev
> +  is the device * for module M. Then this could be achieved by
> +  ret = omap_device_scale(mdev, dev1, f1)
> +  if (ret)
> +  	/* handle error *.

Why exactly is mdev needed in this case?

> +  2. Frequency Throttling
> +  Suppose say there are 2 modules M1 and M2 in Voltage domain VDDx.
> +  Module M1 wants to set VDDx to OPP100 voltage which means M1 and M2 will
> +  be running at OPP100 frequency. Suppose Module M2 wants to run at OPP50
> +  frequency (say f2_opp50) instead of OPP100. This can be achieved by
> +
> +  /* this operation will place M1 and M2 to run at OPP100 */
> +  ret = omap_device_scale(mdev1, dev1, f1_opp100);
> +  if (ret)
> +  	/* handle error *.
> +
> +  /* This operation will bring M2 to run at f2_opp50 w/o decreasing VDDx voltage */
> +  ret = omap_device_scale(mdev2, dev2, f2_opp50);
> +  if (ret)
> +  	/* handle error *.

What are dev1 and dev2 in this example?

Kevin

Re: [PATCH 04/13] OMAP: Introduce dependent voltage domain support

[Kevin Hilman]

Vishwanath BS <vishwanath.bs@ti.com> writes:

> 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.
>
> As part of this, some of the voltage layer structure definitions are moved from
> voltage.c to voltage.h as it needs to be used in the dvfs layer for dependency
> voltage handling.

IMO, it would be cleaner to keep this in the voltage layer, and create
and API for dependencies.  

> Based on original patch from Thara.
>
> Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
> Cc: Thara Gopinath <thara@ti.com>
> ---
>  arch/arm/mach-omap2/dvfs.c                |   87 +++++++++++++++++
>  arch/arm/mach-omap2/voltage.c             |  117 -----------------------
>  arch/arm/plat-omap/include/plat/voltage.h |  148 +++++++++++++++++++++++++++++
>  3 files changed, 235 insertions(+), 117 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/dvfs.c b/arch/arm/mach-omap2/dvfs.c
> index cefc2be..c9d3894 100755
> --- a/arch/arm/mach-omap2/dvfs.c
> +++ b/arch/arm/mach-omap2/dvfs.c
> @@ -85,6 +85,7 @@ struct omap_vdd_dvfs_info {
>  	struct mutex scaling_mutex; /* dvfs mutex */
>  	struct voltagedomain *voltdm;
>  	struct list_head dev_list;
> +	struct device vdd_device;

It's not clear what the usage of this device is for.

It is never initialized, but seems to be used as a dummy device when
calcluating dependencies.   Needs clarification.

Kevin

Re: [PATCH 05/13] OMAP: Introduce device scale implementation

[Kevin Hilman]

Vishwanath BS <vishwanath.bs@ti.com> writes:

> This patch adds omap_device_scale API  which can be used to generic
> device rate scaling.

I would've expected a new omap_device_* API to be part of the
omap_device layer, not added here.

> Based on original patch from Thara.
>
> Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
> Cc: Thara Gopinath <thara@ti.com>
> ---
>  arch/arm/mach-omap2/dvfs.c             |  116 ++++++++++++++++++++++++++++++++
>  arch/arm/plat-omap/include/plat/dvfs.h |    7 ++
>  2 files changed, 123 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/dvfs.c b/arch/arm/mach-omap2/dvfs.c
> index c9d3894..05a9ce3 100755
> --- a/arch/arm/mach-omap2/dvfs.c
> +++ b/arch/arm/mach-omap2/dvfs.c
> @@ -19,6 +19,7 @@
>  #include <plat/common.h>
>  #include <plat/voltage.h>
>  #include <plat/omap_device.h>
> +#include <plat/smartreflex.h>
>  
>  /**
>   * struct omap_dev_user_list - Structure maitain userlist per device
> @@ -585,6 +586,121 @@ static int omap_dvfs_voltage_scale(struct omap_vdd_dvfs_info *dvfs_info)
>  }
>  
>  /**
> + * omap_device_scale() - Set a new rate at which the device is to operate
> + * @req_dev:	pointer to the device requesting the scaling.
> + * @target_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
> + */

Here would be a good place to describe why both the requesting device
and the target device need to be tracked.

> +int omap_device_scale(struct device *req_dev, struct device *target_dev,
> +			unsigned long rate)
> +{
> +	struct opp *opp;
> +	unsigned long volt, freq, min_freq, max_freq;
> +	struct omap_vdd_dvfs_info *dvfs_info;
> +	struct platform_device *pdev;
> +	struct omap_device *od;
> +	int ret = 0;
> +
> +	pdev = container_of(target_dev, struct platform_device, dev);
> +	od = container_of(pdev, struct omap_device, pdev);
> +
> +	/*
> +	 * Figure out if the desired frquency lies between the
> +	 * maximum and minimum possible for the particular device
> +	 */
> +	min_freq = 0;
> +	if (IS_ERR(opp_find_freq_ceil(target_dev, &min_freq))) {
> +		dev_err(target_dev, "%s: Unable to find lowest opp\n",
> +						__func__);
> +		return -ENODEV;
> +	}
> +
> +	max_freq = ULONG_MAX;
> +	if (IS_ERR(opp_find_freq_floor(target_dev, &max_freq))) {
> +		dev_err(target_dev, "%s: Unable to find highest opp\n",
> +						__func__);
> +		return -ENODEV;
> +	}
> +
> +	if (rate < min_freq)
> +		freq = min_freq;
> +	else if (rate > max_freq)
> +		freq = max_freq;
> +	else
> +		freq = rate;
> +

OK, frome here on, I would expect the adjusted value 'freq' to be used
instead of 'rate', but that is not the case below.

> +	opp = opp_find_freq_ceil(target_dev, &freq);
> +	if (IS_ERR(opp)) {
> +		dev_err(target_dev, "%s: Unable to find OPP for freq%ld\n",
> +			__func__, rate);
> +		return -ENODEV;
> +	}
> +
> +	/* 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.
> +	 */

This comment doesn't match the following code.

> +	if (rate) {

Why is rate used here, and not freq?

> +		dvfs_info = get_dvfs_info(od->hwmods[0]->voltdm);
> +
> +		ret = omap_dvfs_add_freq_request(dvfs_info, req_dev,
> +						target_dev, freq);
> +		if (ret) {
> +			dev_err(target_dev, "%s: Unable to add frequency request\n",
> +				__func__);
> +			return ret;
> +		}
> +
> +		ret = omap_dvfs_add_vdd_user(dvfs_info, req_dev, volt);
> +		if (ret) {
> +			dev_err(target_dev, "%s: Unable to add voltage request\n",
> +				__func__);
> +			omap_dvfs_remove_freq_request(dvfs_info, req_dev,
> +				target_dev);
> +			return ret;
> +		}
> +	} else {

The function comments don't describe this case.  Namely, that if you
pass in rate = 0, it removes any previous requests for the requesting
device.

> +		dvfs_info = get_dvfs_info(od->hwmods[0]->voltdm);
> +
> +		ret = omap_dvfs_remove_freq_request(dvfs_info, req_dev,
> +				target_dev);
> +		if (ret) {
> +			dev_err(target_dev, "%s: Unable to remove frequency request\n",
> +				__func__);
> +			return ret;
> +		}
> +
> +		ret = omap_dvfs_remove_vdd_user(dvfs_info, req_dev);
> +		if (ret) {
> +			dev_err(target_dev, "%s: Unable to remove voltage request\n",
> +				__func__);
> +			return ret;
> +		}
> +	}
> +
> +	/* Do the actual scaling */
> +	ret = omap_dvfs_voltage_scale(dvfs_info);

ok

> +	if (!ret)
> +		if (omap_device_get_rate(target_dev) >= rate)
> +			return 0;
> +

but this bit needs some explanation.  IIUC: if the _voltage_scale()
fails (which also scales the frequency) but the frequency was
sucessfully changed, then return success.

Also 'rate' is used here where 'freq' would be expected.

> +	return ret;
> +}
> +EXPORT_SYMBOL(omap_device_scale);
> +
> +/**
>   * omap_dvfs_init() - Initialize omap dvfs layer
>   *
>   * Initalizes omap dvfs layer. It basically allocates memory for
> diff --git a/arch/arm/plat-omap/include/plat/dvfs.h b/arch/arm/plat-omap/include/plat/dvfs.h
> index 1302990..1be2b9d 100644
> --- a/arch/arm/plat-omap/include/plat/dvfs.h
> +++ b/arch/arm/plat-omap/include/plat/dvfs.h
> @@ -17,11 +17,18 @@
>  
>  #ifdef CONFIG_PM
>  int omap_dvfs_register_device(struct voltagedomain *voltdm, struct device *dev);
> +int omap_device_scale(struct device *req_dev, struct device *dev,
> +			unsigned long rate);
>  #else
>  static inline int omap_dvfs_register_device(struct voltagedomain *voltdm,
>  		struct device *dev)
>  {
>  	return -EINVAL;
>  }
> +static inline int omap_device_scale(struct device *req_dev, struct devices
> +			*target_dev, unsigned long rate);
> +{
> +	return -EINVAL;
> +}
>  #endif
>  #endif

Kevin

Re: [PATCH 06/13] OMAP: Disable Smartreflex across DVFS

[Kevin Hilman]

Vishwanath BS <vishwanath.bs@ti.com> writes:

> From: Thara Gopinath <thara@ti.com>
>
> 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.

Should also describe why.

> Signed-off-by: Thara Gopinath <thara@ti.com>
> Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
> ---
>  arch/arm/mach-omap2/dvfs.c |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/dvfs.c b/arch/arm/mach-omap2/dvfs.c
> index 05a9ce3..1e5492c 100755
> --- a/arch/arm/mach-omap2/dvfs.c
> +++ b/arch/arm/mach-omap2/dvfs.c
> @@ -529,6 +529,9 @@ static int omap_dvfs_voltage_scale(struct omap_vdd_dvfs_info *dvfs_info)
>  
>  	curr_volt = omap_voltage_get_nom_volt(voltdm);
>  
> +	/* Disable smartreflex module across voltage and frequency scaling */

comment redundant

> +	omap_sr_disable(voltdm);
> +
>  	if (curr_volt == volt) {
>  		is_volt_scaled = 1;
>  	} else if (curr_volt < volt) {
> @@ -536,6 +539,7 @@ static int omap_dvfs_voltage_scale(struct omap_vdd_dvfs_info *dvfs_info)
>  		if (ret) {
>  			pr_warning("%s: Unable to scale the %s to %ld volt\n",
>  						__func__, voltdm->name, volt);
> +			omap_sr_enable(voltdm);

Would probably be cleaner to make this error path 'goto' the end where
the SR enable and mutex_unlock are already being done.

>  			mutex_unlock(&dvfs_info->scaling_mutex);
>  			return ret;
>  		}
> @@ -570,6 +574,9 @@ static int omap_dvfs_voltage_scale(struct omap_vdd_dvfs_info *dvfs_info)
>  	if (!is_volt_scaled && !ret)
>  		omap_voltage_scale_vdd(voltdm, volt);
>  
> +	/* Enable Smartreflex module */

comment redundant

> +	omap_sr_enable(voltdm);
> +
>  	mutex_unlock(&dvfs_info->scaling_mutex);
>  
>  	/* calculate the voltages for dependent vdd's */

Kevin

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

[Kevin Hilman]

Vishwanath BS <vishwanath.bs@ti.com> writes:

> From: Thara Gopinath <thara@ti.com>
>
> 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 omap_device_populate_rate_fns during system init to register
> various set_rate and get_rate APIs with the omap device layer
>
> Signed-off-by: Thara Gopinath <thara@ti.com>
> ---
>  arch/arm/mach-omap2/pm.c |   71 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 71 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
> index d5a102c..1b94ad4 100644
> --- a/arch/arm/mach-omap2/pm.c
> +++ b/arch/arm/mach-omap2/pm.c
> @@ -23,6 +23,9 @@
>  #include "powerdomain.h"
>  #include "clockdomain.h"
>  #include "pm.h"
> +#include "cm2xxx_3xxx.h"
> +#include "cm-regbits-34xx.h"
> +#include "prm.h"
>  
>  static struct omap_device_pm_latency *pm_lats;
>  
> @@ -31,6 +34,8 @@ static struct device *iva_dev;
>  static struct device *l3_dev;
>  static struct device *dsp_dev;
>  
> +static struct clk *dpll1_clk, *dpll2_clk, *dpll3_clk;
> +
>  struct device *omap2_get_mpuss_device(void)
>  {
>  	WARN_ON_ONCE(!mpu_dev);
> @@ -77,6 +82,55 @@ static int _init_omap_device(char *name, struct device **new_dev)
>  	return 0;
>  }
>  
> +static unsigned long omap3_mpu_get_rate(struct device *dev)
> +{
> +	return dpll1_clk->rate;
> +}
> +
> +static int omap3_mpu_set_rate(struct device *dev, unsigned long rate)
> +{
> +	int ret;
> +
> +	ret = clk_set_rate(dpll1_clk, rate);
> +	if (ret) {
> +		dev_warn(dev, "%s: Unable to set rate to %ld\n",
> +			__func__, rate);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +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 = omap2_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 = omap2_cm_read_mod_reg(CORE_MOD, CM_CLKSEL) &
> +			OMAP3430_CLKSEL_L3_MASK;
> +	return dpll3_clk->rate / l3_div;
> +}
> +
> +
>  /*
>   * Build omap_devices for processors and bus.
>   */
> @@ -90,6 +144,23 @@ static void omap2_init_processor_devices(void)
>  	} else {
>  		_init_omap_device("l3_main", &l3_dev);
>  	}
> +
> +	if (cpu_is_omap34xx()) {
> +		dpll1_clk = clk_get(NULL, "dpll1_ck");
> +		dpll2_clk = clk_get(NULL, "dpll2_ck");
> +		dpll3_clk = clk_get(NULL, "dpll3_m2_ck");

Should check return value of clk_get() calls.

> +		if (mpu_dev)
> +			omap_device_register_dvfs_callbacks(mpu_dev,
> +				omap3_mpu_set_rate, omap3_mpu_get_rate);
> +		if (iva_dev)
> +			omap_device_register_dvfs_callbacks(iva_dev,
> +				omap3_iva_set_rate, omap3_iva_get_rate);
> +		if (l3_dev)
> +			omap_device_register_dvfs_callbacks(l3_dev,
> +				omap3_l3_set_rate, omap3_l3_get_rate);
> +
> +	}
>  }
>  
>  /* Types of sleep_switch used in omap_set_pwrdm_state */

Kevin

Re: [PATCH 08/13] OMAP3: cpufreq driver changes for DVFS support

[Kevin Hilman]

Vishwanath BS <vishwanath.bs@ti.com> writes:

> Changes in the omap cpufreq driver for DVFS support.

Missing descriptive changelog.

Should describe what, why, etc.

Kevin

> Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> ---
>  arch/arm/plat-omap/cpu-omap.c |   35 ++++++++++++++++++++++++++++++++---
>  1 files changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/plat-omap/cpu-omap.c b/arch/arm/plat-omap/cpu-omap.c
> index 1c1b80b..d965220 100644
> --- a/arch/arm/plat-omap/cpu-omap.c
> +++ b/arch/arm/plat-omap/cpu-omap.c
> @@ -30,10 +30,12 @@
>  #include <mach/hardware.h>
>  #include <plat/clock.h>
>  #include <asm/system.h>
> +#include <asm/cpu.h>
>  
>  #if defined(CONFIG_ARCH_OMAP3) && !defined(CONFIG_OMAP_PM_NONE)
>  #include <plat/omap-pm.h>
>  #include <plat/common.h>
> +#include <plat/dvfs.h>
>  #endif
>  
>  #define VERY_HI_RATE	900000000
> @@ -85,11 +87,11 @@ static int omap_target(struct cpufreq_policy *policy,
>  		       unsigned int target_freq,
>  		       unsigned int relation)
>  {
> -#ifdef CONFIG_ARCH_OMAP1
>  	struct cpufreq_freqs freqs;
> -#endif
>  #if defined(CONFIG_ARCH_OMAP3) && !defined(CONFIG_OMAP_PM_NONE)
>  	unsigned long freq;
> +	int i;
> +	struct cpufreq_freqs freqs_notify;
>  	struct device *mpu_dev = omap2_get_mpuss_device();
>  #endif
>  	int ret = 0;
> @@ -116,9 +118,36 @@ static int omap_target(struct cpufreq_policy *policy,
>  	ret = clk_set_rate(mpu_clk, freqs.new * 1000);
>  	cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
>  #elif defined(CONFIG_ARCH_OMAP3) && !defined(CONFIG_OMAP_PM_NONE)
> +	freqs.old = omap_getspeed(policy->cpu);;
> +	freqs_notify.new = clk_round_rate(mpu_clk, target_freq * 1000) / 1000;
> +	freqs.cpu = policy->cpu;
> +
> +	if (freqs.old == freqs.new)
> +		return ret;
> +
> +	/* pre notifiers */
> +	for_each_cpu(i, policy->cpus) {
> +		freqs.cpu = i;
> +		cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
> +	}
> +
> +	/* scale the frequency */
>  	freq = target_freq * 1000;
>  	if (opp_find_freq_ceil(mpu_dev, &freq))
> -		omap_pm_cpu_set_freq(freq);
> +		omap_device_scale(mpu_dev, mpu_dev, freq);
> +
> +	/* Update loops per jiffy */
> +	freqs.new = omap_getspeed(policy->cpu);
> +	for_each_cpu(i, policy->cpus)
> +		per_cpu(cpu_data, i).loops_per_jiffy =
> +		cpufreq_scale(per_cpu(cpu_data, i).loops_per_jiffy,
> +				freqs.old, freqs.new);
> +
> +	/* post notifiers */
> +	for_each_cpu(i, policy->cpus) {
> +		freqs.cpu = i;
> +		cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
> +	}
>  #endif
>  	return ret;
>  }

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

[Kevin Hilman]

Vishwanath BS <vishwanath.bs@ti.com> writes:

> From: Thara Gopinath <thara@ti.com>
>
> This patch adds voltage domain info in the relevant
> device hwmod structures so as to enable OMAP3 DVFS
> support.

Has there been work yet on the autogen scripts to auto-generate this for
OMAP4?

Kevin

> 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 8d81813..c57f34d 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
> @@ -94,6 +94,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,
> @@ -384,6 +385,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),
> @@ -412,6 +414,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)

Re: [PATCH 11/13] OMAP2PLUS: Replace voltage values with Macros

[Kevin Hilman]

Vishwanath BS <vishwanath.bs@ti.com> writes:

> Currently voltage values on opp tables are hardcoded. As these voltage values
> are anyway defined in voltage.h as macros, opp table can reuse these values.
> This will avoid opp table and voltage layer having conflicting values.

ok

> Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
> This patch has 2 line over 80 char warning. This is kept for readability
> purpose.

This comment should be after the '---'

Kevin

> ---
>  arch/arm/mach-omap2/opp3xxx_data.c |   47 ++++++++++++++++++-----------------
>  arch/arm/mach-omap2/opp4xxx_data.c |   13 +++++----
>  2 files changed, 31 insertions(+), 29 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/opp3xxx_data.c b/arch/arm/mach-omap2/opp3xxx_data.c
> index 0486fce..eefd6af 100644
> --- a/arch/arm/mach-omap2/opp3xxx_data.c
> +++ b/arch/arm/mach-omap2/opp3xxx_data.c
> @@ -19,20 +19,21 @@
>  #include <linux/module.h>
>  
>  #include <plat/cpu.h>
> +#include <plat/voltage.h>
>  
>  #include "omap_opp_data.h"
>  
>  static struct omap_opp_def __initdata omap34xx_opp_def_list[] = {
>  	/* MPU OPP1 */
> -	OPP_INITIALIZER("mpu", true, 125000000, 975000),
> +	OPP_INITIALIZER("mpu", true, 125000000, OMAP3430_VDD_MPU_OPP1_UV),
>  	/* MPU OPP2 */
> -	OPP_INITIALIZER("mpu", true, 250000000, 1075000),
> +	OPP_INITIALIZER("mpu", true, 250000000, OMAP3430_VDD_MPU_OPP2_UV),
>  	/* MPU OPP3 */
> -	OPP_INITIALIZER("mpu", true, 500000000, 1200000),
> +	OPP_INITIALIZER("mpu", true, 500000000, OMAP3430_VDD_MPU_OPP3_UV),
>  	/* MPU OPP4 */
> -	OPP_INITIALIZER("mpu", true, 550000000, 1270000),
> +	OPP_INITIALIZER("mpu", true, 550000000, OMAP3430_VDD_MPU_OPP4_UV),
>  	/* MPU OPP5 */
> -	OPP_INITIALIZER("mpu", true, 600000000, 1350000),
> +	OPP_INITIALIZER("mpu", true, 600000000, OMAP3430_VDD_MPU_OPP5_UV),
>  
>  	/*
>  	 * L3 OPP1 - 41.5 MHz is disabled because: The voltage for that OPP is
> @@ -42,47 +43,47 @@ static struct omap_opp_def __initdata omap34xx_opp_def_list[] = {
>  	 * impact that frequency will do to the MPU and the whole system in
>  	 * general.
>  	 */
> -	OPP_INITIALIZER("l3_main", false, 41500000, 975000),
> +	OPP_INITIALIZER("l3_main", false, 41500000, OMAP3430_VDD_CORE_OPP1_UV),
>  	/* L3 OPP2 */
> -	OPP_INITIALIZER("l3_main", true, 83000000, 1050000),
> +	OPP_INITIALIZER("l3_main", true, 83000000, OMAP3430_VDD_CORE_OPP2_UV),
>  	/* L3 OPP3 */
> -	OPP_INITIALIZER("l3_main", true, 166000000, 1150000),
> +	OPP_INITIALIZER("l3_main", true, 166000000, OMAP3430_VDD_CORE_OPP3_UV),
>  
>  	/* DSP OPP1 */
> -	OPP_INITIALIZER("iva", true, 90000000, 975000),
> +	OPP_INITIALIZER("iva", true, 90000000, OMAP3430_VDD_MPU_OPP1_UV),
>  	/* DSP OPP2 */
> -	OPP_INITIALIZER("iva", true, 180000000, 1075000),
> +	OPP_INITIALIZER("iva", true, 180000000, OMAP3430_VDD_MPU_OPP2_UV),
>  	/* DSP OPP3 */
> -	OPP_INITIALIZER("iva", true, 360000000, 1200000),
> +	OPP_INITIALIZER("iva", true, 360000000, OMAP3430_VDD_MPU_OPP3_UV),
>  	/* DSP OPP4 */
> -	OPP_INITIALIZER("iva", true, 400000000, 1270000),
> +	OPP_INITIALIZER("iva", true, 400000000, OMAP3430_VDD_MPU_OPP4_UV),
>  	/* DSP OPP5 */
> -	OPP_INITIALIZER("iva", true, 430000000, 1350000),
> +	OPP_INITIALIZER("iva", true, 430000000, OMAP3430_VDD_MPU_OPP5_UV),
>  };
>  
>  static struct omap_opp_def __initdata omap36xx_opp_def_list[] = {
>  	/* MPU OPP1 - OPP50 */
> -	OPP_INITIALIZER("mpu", true,  300000000, 1012500),
> +	OPP_INITIALIZER("mpu", true,  300000000, OMAP3630_VDD_MPU_OPP50_UV),
>  	/* MPU OPP2 - OPP100 */
> -	OPP_INITIALIZER("mpu", true,  600000000, 1200000),
> +	OPP_INITIALIZER("mpu", true,  600000000, OMAP3630_VDD_MPU_OPP100_UV),
>  	/* MPU OPP3 - OPP-Turbo */
> -	OPP_INITIALIZER("mpu", false, 800000000, 1325000),
> +	OPP_INITIALIZER("mpu", false, 800000000, OMAP3630_VDD_MPU_OPP120_UV),
>  	/* MPU OPP4 - OPP-SB */
> -	OPP_INITIALIZER("mpu", false, 1000000000, 1375000),
> +	OPP_INITIALIZER("mpu", false, 1000000000, OMAP3630_VDD_MPU_OPP1G_UV),
>  
>  	/* L3 OPP1 - OPP50 */
> -	OPP_INITIALIZER("l3_main", true, 100000000, 1000000),
> +	OPP_INITIALIZER("l3_main", true, 100000000, OMAP3630_VDD_CORE_OPP50_UV),
>  	/* L3 OPP2 - OPP100, OPP-Turbo, OPP-SB */
> -	OPP_INITIALIZER("l3_main", true, 200000000, 1200000),
> +	OPP_INITIALIZER("l3_main", true, 200000000, OMAP3630_VDD_CORE_OPP100_UV),
>  
>  	/* DSP OPP1 - OPP50 */
> -	OPP_INITIALIZER("iva", true,  260000000, 1012500),
> +	OPP_INITIALIZER("iva", true,  260000000, OMAP3630_VDD_MPU_OPP50_UV),
>  	/* DSP OPP2 - OPP100 */
> -	OPP_INITIALIZER("iva", true,  520000000, 1200000),
> +	OPP_INITIALIZER("iva", true,  520000000, OMAP3630_VDD_MPU_OPP100_UV),
>  	/* DSP OPP3 - OPP-Turbo */
> -	OPP_INITIALIZER("iva", false, 660000000, 1325000),
> +	OPP_INITIALIZER("iva", false, 660000000, OMAP3630_VDD_MPU_OPP120_UV),
>  	/* DSP OPP4 - OPP-SB */
> -	OPP_INITIALIZER("iva", false, 800000000, 1375000),
> +	OPP_INITIALIZER("iva", false, 800000000, OMAP3630_VDD_MPU_OPP1G_UV),
>  };
>  
>  /**
> diff --git a/arch/arm/mach-omap2/opp4xxx_data.c b/arch/arm/mach-omap2/opp4xxx_data.c
> index a11fa56..80d08bc 100644
> --- a/arch/arm/mach-omap2/opp4xxx_data.c
> +++ b/arch/arm/mach-omap2/opp4xxx_data.c
> @@ -20,22 +20,23 @@
>  #include <linux/module.h>
>  
>  #include <plat/cpu.h>
> +#include <plat/voltage.h>
>  
>  #include "omap_opp_data.h"
>  
>  static struct omap_opp_def __initdata omap44xx_opp_def_list[] = {
>  	/* MPU OPP1 - OPP50 */
> -	OPP_INITIALIZER("mpu", true, 300000000, 1100000),
> +	OPP_INITIALIZER("mpu", true, 300000000, OMAP4430_VDD_MPU_OPP50_UV),
>  	/* MPU OPP2 - OPP100 */
> -	OPP_INITIALIZER("mpu", true, 600000000, 1200000),
> +	OPP_INITIALIZER("mpu", true, 600000000, OMAP4430_VDD_MPU_OPP100_UV),
>  	/* MPU OPP3 - OPP-Turbo */
> -	OPP_INITIALIZER("mpu", false, 800000000, 1260000),
> +	OPP_INITIALIZER("mpu", false, 800000000, OMAP4430_VDD_MPU_OPPTURBO_UV),
>  	/* MPU OPP4 - OPP-SB */
> -	OPP_INITIALIZER("mpu", false, 1008000000, 1350000),
> +	OPP_INITIALIZER("mpu", false, 1008000000, OMAP4430_VDD_MPU_OPPNITRO_UV),
>  	/* L3 OPP1 - OPP50 */
> -	OPP_INITIALIZER("l3_main_1", true, 100000000, 930000),
> +	OPP_INITIALIZER("l3_main_1", true, 100000000, OMAP4430_VDD_CORE_OPP50_UV),
>  	/* L3 OPP2 - OPP100, OPP-Turbo, OPP-SB */
> -	OPP_INITIALIZER("l3_main_1", true, 200000000, 1100000),
> +	OPP_INITIALIZER("l3_main_1", true, 200000000, OMAP4430_VDD_CORE_OPP100_UV),
>  	/* TODO: add IVA, DSP, aess, fdif, gpu */
>  };

RE: [PATCH 02/13] OMAP: Introduce device specific set rate and get rate in omap_device structure

[Vishwanath Sripathy]

> -----Original Message-----
> From: Kevin Hilman [mailto:khilman@ti.com]
> Sent: Friday, February 04, 2011 5:17 AM
> To: Vishwanath BS
> Cc: linux-omap@vger.kernel.org; patches@linaro.org; Thara Gopinath
> Subject: Re: [PATCH 02/13] OMAP: Introduce device specific set rate and
> get rate in omap_device structure
>
> Vishwanath BS <vishwanath.bs@ti.com> writes:
>
> > From: Thara Gopinath <thara@ti.com>
> >
> > This patch extends the omap_device 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 omap device layer
> > namely omap_device_set_rate that can be called to set a new
> operating
> > rate for a device, omap_device_get_rate that can be called to retrieve
> > the operating frequency for a device and
> omap_device_register_dvfs_callbacks
> > to register the device specific set_rate and get_rate functions.
> > The omap_device_set_rate and omap_device_get_rate does some
> routine error
> > checks and finally calls into the device specific set_rate
> > and get_rate APIs populated through omap_device_populate_rate_fns.
> >
> > Signed-off-by: Thara Gopinath <thara@ti.com>
> > Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
> > ---
> >  arch/arm/plat-omap/include/plat/omap_device.h |    9 +++++
> >  arch/arm/plat-omap/omap_device.c              |   49
> +++++++++++++++++++++++++
> >  2 files changed, 58 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 e4c349f..204fb0a 100644
> > --- a/arch/arm/plat-omap/include/plat/omap_device.h
> > +++ b/arch/arm/plat-omap/include/plat/omap_device.h
> > @@ -50,6 +50,8 @@ extern struct device omap_device_parent;
> >   * @hwmods: (one .. many per omap_device)
> >   * @hwmods_cnt: ARRAY_SIZE() of @hwmods
> >   * @pm_lats: ptr to an omap_device_pm_latency table
> > + * @set_rate: fn ptr to change the operating rate.
> > + * @get_rate: fn ptr to retrieve the current operating rate.
> >   * @pm_lats_cnt: ARRAY_SIZE() of what is passed to @pm_lats
> >   * @pm_lat_level: array index of the last odpl entry executed - -1 if
> never
> >   * @dev_wakeup_lat: dev wakeup latency in nanoseconds
> > @@ -73,6 +75,8 @@ struct omap_device {
> >  	s8				pm_lat_level;
> >  	u8				hwmods_cnt;
> >  	u8				_state;
> > +	int (*set_rate)(struct device *dev, unsigned long rate);
> > +	unsigned long (*get_rate) (struct device *dev);
> >  };
> >
> >  /* Device driver interface (call via platform_data fn ptrs) */
> > @@ -107,6 +111,11 @@ void __iomem
> *omap_device_get_rt_va(struct omap_device *od);
> >  int omap_device_align_pm_lat(struct platform_device *pdev,
> >  			     u32 new_wakeup_lat_limit);
> >  struct powerdomain *omap_device_get_pwrdm(struct omap_device
> *od);
> > +int omap_device_set_rate(struct device *dev, unsigned long freq);
> > +unsigned long omap_device_get_rate(struct device *dev);
> > +void omap_device_register_dvfs_callbacks(struct device *dev,
> > +		int (*set_rate)(struct device *dev, unsigned long rate),
> > +		unsigned long (*get_rate) (struct device *dev));
> >  u32 omap_device_get_context_loss_count(struct platform_device
> *pdev);
> >
> >  /* Other */
> > diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-
> omap/omap_device.c
> > index a84e849..4cee430 100644
> > --- a/arch/arm/plat-omap/omap_device.c
> > +++ b/arch/arm/plat-omap/omap_device.c
> > @@ -810,6 +810,55 @@ int omap_device_enable_clocks(struct
> omap_device *od)
> >  	return 0;
> >  }
> >
> > +int omap_device_set_rate(struct device *dev, unsigned long freq)
> > +{
> > +	struct platform_device *pdev;
> > +	struct omap_device *od;
> > +
> > +	pdev = container_of(dev, struct platform_device, dev);
>
> There is a macro for this in device.h:
>
>       pdev = to_platform_device(dev)
>
> This needs to be fixed all over this series.
OK. Will fix it in next vesion.
>
> > +	od = _find_by_pdev(pdev);
> > +
> > +	if (!od->set_rate) {
> > +		dev_err(dev, "%s: No set_rate API for scaling device\n",
> > +			__func__);
> > +		return -ENODATA;
> > +	}
> > +
> > +	return od->set_rate(dev, freq);
> > +}
> > +
> > +unsigned long omap_device_get_rate(struct device *dev)
> > +{
> > +	struct platform_device *pdev;
> > +	struct omap_device *od;
> > +
> > +	pdev = container_of(dev, struct platform_device, dev);
> > +	od = _find_by_pdev(pdev);
>
> Should also check here for a valid omap_device, but making sure its
> parent is omap_device_parent.
OK
>
> > +
> > +
> > +	if (!od->get_rate) {
> > +		dev_err(dev, "%s: No get rate API for the device\n",
> > +			__func__);
> > +		return 0;
> > +	}
> > +
> > +	return od->get_rate(dev);
> > +}
> > +
> > +void omap_device_register_dvfs_callbacks(struct device *dev,
> > +		int (*set_rate)(struct device *dev, unsigned long rate),
> > +		unsigned long (*get_rate) (struct device *dev))
> > +{
> > +	struct platform_device *pdev;
> > +	struct omap_device *od;
> > +
> > +	pdev = container_of(dev, struct platform_device, dev);
> > +	od = _find_by_pdev(pdev);
> > +
>
> and here
OK

Vishwa
>
> > +	od->set_rate = set_rate;
> > +	od->get_rate = get_rate;
> > +}
> > +
> >  struct device omap_device_parent = {
> >  	.init_name	= "omap",
> >  	.parent         = &platform_bus,
>
> Kevin

RE: [PATCH 03/13] OMAP: Implement Basic DVFS

[Vishwanath Sripathy]

> -----Original Message-----
> From: Kevin Hilman [mailto:khilman@ti.com]
> Sent: Friday, February 04, 2011 6:44 AM
> To: Vishwanath BS
> Cc: linux-omap@vger.kernel.org; patches@linaro.org; Thara Gopinath
> Subject: Re: [PATCH 03/13] OMAP: Implement Basic DVFS
>
> Vishwanath BS <vishwanath.bs@ti.com> writes:
>
> > This patch introduces an API to perform DVFS for a given voltage
> domain.
> > It takes omap_vdd_dvfs_info pointer as input parameter, computes
> the highest
> > requested voltage for that vdd and scales all the devices in that vdd
to
> the
> > requested frequency along with voltage scaling.
> >
> > Based on original patch from Thara.
> >
> > Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
> > Cc: Thara Gopinath <thara@ti.com>
> > ---
> >  arch/arm/mach-omap2/dvfs.c |   87
> +++++++++++++++++++++++++++++++++++++++++++-
> >  1 files changed, 86 insertions(+), 1 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/dvfs.c b/arch/arm/mach-
> omap2/dvfs.c
> > index 8832e4a..cefc2be 100755
> > --- a/arch/arm/mach-omap2/dvfs.c
> > +++ b/arch/arm/mach-omap2/dvfs.c
> > @@ -21,7 +21,7 @@
> >  #include <plat/omap_device.h>
> >
> >  /**
> > - * struct omap_dev_user_list - Structure maitain userlist per devide
> > + * struct omap_dev_user_list - Structure maitain userlist per device
>
> this typo should be done in the original patch, not here.
OK
>
> >   *
> >   * @dev:       The device requesting for a particular frequency
> >   * @node:      The list head entry
> > @@ -413,6 +413,91 @@ static int
> omap_dvfs_remove_freq_request(struct omap_vdd_dvfs_info *dvfs_info,
> >  }
> >
> >  /**
> > + * omap_dvfs_voltage_scale() : API to scale the devices associated
> with a
> > + *						voltage domain vdd
voltage.
>
> This function scales both voltage and frequency, so the name
> voltage_scale() is a bit misleading.
Does omap_dvfs_do_dvfs look good?
>
> > + * @dvfs_info: omap_vdd_dvfs_info pointer for the required vdd
> > + *
> > + * This API runs through the list of devices associated with the
> > + * voltage domain and scales the device rates to the one requested
> > + * by the user or those corresponding to the new voltage of the
> > + * voltage domain. Target voltage is the highest voltage in the
> vdd_user_list.
> > + *
> > + * Returns 0 on success
> > + * else the error value.
> > + */
> > +static int omap_dvfs_voltage_scale(struct omap_vdd_dvfs_info
> *dvfs_info)
> > +{
> > +	unsigned long curr_volt;
> > +	int is_volt_scaled = 0;
>
> should be a bool
ok
>
> > +	struct omap_vdd_dev_list *temp_dev;
> > +	struct plist_node *node;
> > +	int ret = 0;
> > +	struct voltagedomain *voltdm;
> > +	unsigned long volt;
> > +
> > +	if (!dvfs_info || IS_ERR(dvfs_info)) {
> > +		pr_warning("%s: VDD specified does not exist!\n",
> __func__);
> > +		return -EINVAL;
> > +	}
> > +
> > +	voltdm = dvfs_info->voltdm;
> > +
> > +	mutex_lock(&dvfs_info->scaling_mutex);
> > +
> > +	/* Find the highest voltage being requested */
> > +	node = plist_last(&dvfs_info->user_list);
> > +	volt = node->prio;
> > +
> > +	curr_volt = omap_voltage_get_nom_volt(voltdm);
> > +
> > +	if (curr_volt == volt) {
> > +		is_volt_scaled = 1;
> > +	} else if (curr_volt < volt) {
> > +		ret = omap_voltage_scale_vdd(voltdm, volt);
> > +		if (ret) {
> > +			pr_warning("%s: Unable to scale the %s to %ld
> volt\n",
> > +						__func__, voltdm->name,
> volt);
> > +			mutex_unlock(&dvfs_info->scaling_mutex);
> > +			return ret;
> > +		}
> > +		is_volt_scaled = 1;
> > +	}
> > +
> > +	list_for_each_entry(temp_dev, &dvfs_info->dev_list, node) {
> > +		struct device *dev;
> > +		struct opp *opp;
> > +		unsigned long freq;
> > +
> > +		dev = temp_dev->dev;
>
> if you're doing this assignment here, might as well make 'dev' the
> iterator instead of temp_dev.
temp_dev holds pointer to omap_vdd_dev_list where as dev points to actual
device pointer. Hence this assignment.
>
> This section would benefit with some comments.  If I understand the
> code
> correctly, something like:
>
> If a frequency has been requested, use the highest requested frequency.
>
> > +		if (!plist_head_empty(&temp_dev->user_list)) {
> > +			node = plist_last(&temp_dev->user_list);
> > +			freq = node->prio;
>
> otherwise check if device has OPP for this voltage
>
> > +		} else {
> > +			opp = omap_dvfs_find_voltage(dev, volt);
> > +			if (IS_ERR(opp))
> > +				continue;
>
> This needs a comment to, but I'm not sure I understand what's going on
> here.  What it seems like:
>
> if this device has no OPP for this voltage, just silently move on to the
> next device?   doesn't seem quite right, but not sure I fully grok the
> failure modes of omap_dvfs_find_voltage()
Yes, your understanding is right. omap_dvfs_find_voltage will return error
if the device does not have an OPP table.
Typically devices should not register with a vdd (using
omap_dvfs_register_device) if it has no OPP table associated with it. So
ideally we should not hit this error case. But only exception so far is SR
driver. SR hwmod has vdd_name field set so as to get voltagedomain
pointers. But SR does not have any opp table. So there is no harm in
ignoring the error and moving to next device.

Vishwa
>
> Kevin
>
> > +			freq = opp_get_freq(opp);
> > +		}
> > +
> > +		if (freq == omap_device_get_rate(dev)) {
> > +			dev_dbg(dev, "%s: Already at the requested"
> > +				"rate %ld\n", __func__, freq);
> > +			continue;
> > +		}
> > +
> > +		ret |= omap_device_set_rate(dev, freq);
> > +	}
> > +
> > +	if (!is_volt_scaled && !ret)
> > +		omap_voltage_scale_vdd(voltdm, volt);
> > +
> > +	mutex_unlock(&dvfs_info->scaling_mutex);
> > +
> > +	return 0;
> > +}
> > +
> > +/**
> >   * omap_dvfs_init() - Initialize omap dvfs layer
> >   *
> >   * Initalizes omap dvfs layer. It basically allocates memory for

RE: [PATCH 04/13] OMAP: Introduce dependent voltage domain support

[Vishwanath Sripathy]

> -----Original Message-----
> From: Kevin Hilman [mailto:khilman@ti.com]
> Sent: Friday, February 04, 2011 9:07 PM
> To: Vishwanath BS
> Cc: linux-omap@vger.kernel.org; patches@linaro.org; Thara Gopinath
> Subject: Re: [PATCH 04/13] OMAP: Introduce dependent voltage domain
> support
>
> Vishwanath BS <vishwanath.bs@ti.com> writes:
>
> > 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.
> >
> > As part of this, some of the voltage layer structure definitions are
> moved from
> > voltage.c to voltage.h as it needs to be used in the dvfs layer for
> dependency
> > voltage handling.
>
> IMO, it would be cleaner to keep this in the voltage layer, and create
> and API for dependencies.
Dependency voltage handling needs some of dvfs layer functions (like
omap_dvfs_add_vdd_user, omap_dvfs_voltage_scale). Given that dvfs layer is
built on top of voltage layer, if these functions need to be implemented
in voltage layer, then voltage layer will end up using dvfs functions
leading to cross dependencies. So I thought keeping this implementation in
dvfs layer makes it more structured.

>
> > Based on original patch from Thara.
> >
> > Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
> > Cc: Thara Gopinath <thara@ti.com>
> > ---
> >  arch/arm/mach-omap2/dvfs.c                |   87
> +++++++++++++++++
> >  arch/arm/mach-omap2/voltage.c             |  117
-----------------------
> >  arch/arm/plat-omap/include/plat/voltage.h |  148
> +++++++++++++++++++++++++++++
> >  3 files changed, 235 insertions(+), 117 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/dvfs.c b/arch/arm/mach-
> omap2/dvfs.c
> > index cefc2be..c9d3894 100755
> > --- a/arch/arm/mach-omap2/dvfs.c
> > +++ b/arch/arm/mach-omap2/dvfs.c
> > @@ -85,6 +85,7 @@ struct omap_vdd_dvfs_info {
> >  	struct mutex scaling_mutex; /* dvfs mutex */
> >  	struct voltagedomain *voltdm;
> >  	struct list_head dev_list;
> > +	struct device vdd_device;
>
> It's not clear what the usage of this device is for.
>
> It is never initialized, but seems to be used as a dummy device when
> calcluating dependencies.   Needs clarification.
This device is used for placing voltage request (omap_dvfs_add_vdd_user)
when dealing with dependent vdds. Eg: while scaling MPU VDD, we also scale
CORE VDD. So while placing voltage request for CORE VDD, this vdd_device
of MPU is used as the requesting device so that this request is stored as
a separate user.

Vishwa
>
> Kevin

RE: [PATCH 05/13] OMAP: Introduce device scale implementation

[Vishwanath Sripathy]

> -----Original Message-----
> From: Kevin Hilman [mailto:khilman@ti.com]
> Sent: Friday, February 04, 2011 9:35 PM
> To: Vishwanath BS
> Cc: linux-omap@vger.kernel.org; patches@linaro.org; Thara Gopinath
> Subject: Re: [PATCH 05/13] OMAP: Introduce device scale
> implementation
>
> Vishwanath BS <vishwanath.bs@ti.com> writes:
>
> > This patch adds omap_device_scale API  which can be used to generic
> > device rate scaling.
>
> I would've expected a new omap_device_* API to be part of the
> omap_device layer, not added here.
Given that this API does not deal with any of the omap_device layer
functions or data structures, I am not sure if it logically belongs to
omap device layer. If you think it should belong to omap_device layer just
because the name starts with omap_device, I would rather rename this API.

>
> > Based on original patch from Thara.
> >
> > Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
> > Cc: Thara Gopinath <thara@ti.com>
> > ---
> >  arch/arm/mach-omap2/dvfs.c             |  116
> ++++++++++++++++++++++++++++++++
> >  arch/arm/plat-omap/include/plat/dvfs.h |    7 ++
> >  2 files changed, 123 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/dvfs.c b/arch/arm/mach-
> omap2/dvfs.c
> > index c9d3894..05a9ce3 100755
> > --- a/arch/arm/mach-omap2/dvfs.c
> > +++ b/arch/arm/mach-omap2/dvfs.c
> > @@ -19,6 +19,7 @@
> >  #include <plat/common.h>
> >  #include <plat/voltage.h>
> >  #include <plat/omap_device.h>
> > +#include <plat/smartreflex.h>
> >
> >  /**
> >   * struct omap_dev_user_list - Structure maitain userlist per device
> > @@ -585,6 +586,121 @@ static int omap_dvfs_voltage_scale(struct
> omap_vdd_dvfs_info *dvfs_info)
> >  }
> >
> >  /**
> > + * omap_device_scale() - Set a new rate at which the device is to
> operate
> > + * @req_dev:	pointer to the device requesting the scaling.
> > + * @target_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
> > + */
>
> Here would be a good place to describe why both the requesting device
> and the target device need to be tracked.
OK

>
> > +int omap_device_scale(struct device *req_dev, struct device
> *target_dev,
> > +			unsigned long rate)
> > +{
> > +	struct opp *opp;
> > +	unsigned long volt, freq, min_freq, max_freq;
> > +	struct omap_vdd_dvfs_info *dvfs_info;
> > +	struct platform_device *pdev;
> > +	struct omap_device *od;
> > +	int ret = 0;
> > +
> > +	pdev = container_of(target_dev, struct platform_device, dev);
> > +	od = container_of(pdev, struct omap_device, pdev);
> > +
> > +	/*
> > +	 * Figure out if the desired frquency lies between the
> > +	 * maximum and minimum possible for the particular device
> > +	 */
> > +	min_freq = 0;
> > +	if (IS_ERR(opp_find_freq_ceil(target_dev, &min_freq))) {
> > +		dev_err(target_dev, "%s: Unable to find lowest opp\n",
> > +						__func__);
> > +		return -ENODEV;
> > +	}
> > +
> > +	max_freq = ULONG_MAX;
> > +	if (IS_ERR(opp_find_freq_floor(target_dev, &max_freq))) {
> > +		dev_err(target_dev, "%s: Unable to find highest opp\n",
> > +						__func__);
> > +		return -ENODEV;
> > +	}
> > +
> > +	if (rate < min_freq)
> > +		freq = min_freq;
> > +	else if (rate > max_freq)
> > +		freq = max_freq;
> > +	else
> > +		freq = rate;
> > +
>
> OK, frome here on, I would expect the adjusted value 'freq' to be used
> instead of 'rate', but that is not the case below.
>
> > +	opp = opp_find_freq_ceil(target_dev, &freq);
> > +	if (IS_ERR(opp)) {
> > +		dev_err(target_dev, "%s: Unable to find OPP for
> freq%ld\n",
> > +			__func__, rate);
> > +		return -ENODEV;
> > +	}
Freq is appropriate than rate here. Will fix it.
> > +
> > +	/* 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.
> > +	 */
>
> This comment doesn't match the following code.
OK. Will fix it. Copy paste error.
>
> > +	if (rate) {
>
> Why is rate used here, and not freq?
Freq can never be 0. If somebody wants to remove his DVFS request (he does
not really care about the device frequency), then he can pass rate as 0.
Hence rate is used.
>
> > +		dvfs_info = get_dvfs_info(od->hwmods[0]->voltdm);
> > +
> > +		ret = omap_dvfs_add_freq_request(dvfs_info, req_dev,
> > +						target_dev, freq);
> > +		if (ret) {
> > +			dev_err(target_dev, "%s: Unable to add frequency
> request\n",
> > +				__func__);
> > +			return ret;
> > +		}
> > +
> > +		ret = omap_dvfs_add_vdd_user(dvfs_info, req_dev, volt);
> > +		if (ret) {
> > +			dev_err(target_dev, "%s: Unable to add voltage
> request\n",
> > +				__func__);
> > +			omap_dvfs_remove_freq_request(dvfs_info,
> req_dev,
> > +				target_dev);
> > +			return ret;
> > +		}
> > +	} else {
>
> The function comments don't describe this case.  Namely, that if you
> pass in rate = 0, it removes any previous requests for the requesting
> device.
OK. Will add that in function comments.
>
> > +		dvfs_info = get_dvfs_info(od->hwmods[0]->voltdm);
> > +
> > +		ret = omap_dvfs_remove_freq_request(dvfs_info,
> req_dev,
> > +				target_dev);
> > +		if (ret) {
> > +			dev_err(target_dev, "%s: Unable to remove
> frequency request\n",
> > +				__func__);
> > +			return ret;
> > +		}
> > +
> > +		ret = omap_dvfs_remove_vdd_user(dvfs_info, req_dev);
> > +		if (ret) {
> > +			dev_err(target_dev, "%s: Unable to remove voltage
> request\n",
> > +				__func__);
> > +			return ret;
> > +		}
> > +	}
> > +
> > +	/* Do the actual scaling */
> > +	ret = omap_dvfs_voltage_scale(dvfs_info);
>
> ok
>
> > +	if (!ret)
> > +		if (omap_device_get_rate(target_dev) >= rate)
> > +			return 0;
> > +
>
> but this bit needs some explanation.  IIUC: if the _voltage_scale()
> fails (which also scales the frequency) but the frequency was
> sucessfully changed, then return success.
Yes, this is basically to cater for situations where some of the devices
in the associated vdd have locked their frequencies. In that case, voltage
scaling would not have happened where as frequency for the requested
device has been set successfully. In that case return success. Will add
these in function comments.
>
> Also 'rate' is used here where 'freq' would be expected.
OK. Will fix it.

Vishwa
>
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL(omap_device_scale);
> > +
> > +/**
> >   * omap_dvfs_init() - Initialize omap dvfs layer
> >   *
> >   * Initalizes omap dvfs layer. It basically allocates memory for
> > diff --git a/arch/arm/plat-omap/include/plat/dvfs.h b/arch/arm/plat-
> omap/include/plat/dvfs.h
> > index 1302990..1be2b9d 100644
> > --- a/arch/arm/plat-omap/include/plat/dvfs.h
> > +++ b/arch/arm/plat-omap/include/plat/dvfs.h
> > @@ -17,11 +17,18 @@
> >
> >  #ifdef CONFIG_PM
> >  int omap_dvfs_register_device(struct voltagedomain *voltdm, struct
> device *dev);
> > +int omap_device_scale(struct device *req_dev, struct device *dev,
> > +			unsigned long rate);
> >  #else
> >  static inline int omap_dvfs_register_device(struct voltagedomain
> *voltdm,
> >  		struct device *dev)
> >  {
> >  	return -EINVAL;
> >  }
> > +static inline int omap_device_scale(struct device *req_dev, struct
> devices
> > +			*target_dev, unsigned long rate);
> > +{
> > +	return -EINVAL;
> > +}
> >  #endif
> >  #endif
>
> Kevin

RE: [PATCH 06/13] OMAP: Disable Smartreflex across DVFS

[Vishwanath Sripathy]

> -----Original Message-----
> From: Kevin Hilman [mailto:khilman@ti.com]
> Sent: Friday, February 04, 2011 9:36 PM
> To: Vishwanath BS
> Cc: linux-omap@vger.kernel.org; patches@linaro.org; Thara Gopinath
> Subject: Re: [PATCH 06/13] OMAP: Disable Smartreflex across DVFS
>
> Vishwanath BS <vishwanath.bs@ti.com> writes:
>
> > From: Thara Gopinath <thara@ti.com>
> >
> > 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.
>
> Should also describe why.
>
> > Signed-off-by: Thara Gopinath <thara@ti.com>
> > Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
> > ---
> >  arch/arm/mach-omap2/dvfs.c |    7 +++++++
> >  1 files changed, 7 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/dvfs.c b/arch/arm/mach-
> omap2/dvfs.c
> > index 05a9ce3..1e5492c 100755
> > --- a/arch/arm/mach-omap2/dvfs.c
> > +++ b/arch/arm/mach-omap2/dvfs.c
> > @@ -529,6 +529,9 @@ static int omap_dvfs_voltage_scale(struct
> omap_vdd_dvfs_info *dvfs_info)
> >
> >  	curr_volt = omap_voltage_get_nom_volt(voltdm);
> >
> > +	/* Disable smartreflex module across voltage and frequency
> scaling */
>
> comment redundant
>
> > +	omap_sr_disable(voltdm);
> > +
> >  	if (curr_volt == volt) {
> >  		is_volt_scaled = 1;
> >  	} else if (curr_volt < volt) {
> > @@ -536,6 +539,7 @@ static int omap_dvfs_voltage_scale(struct
> omap_vdd_dvfs_info *dvfs_info)
> >  		if (ret) {
> >  			pr_warning("%s: Unable to scale the %s to %ld
> volt\n",
> >  						__func__, voltdm->name,
> volt);
> > +			omap_sr_enable(voltdm);
>
> Would probably be cleaner to make this error path 'goto' the end where
> the SR enable and mutex_unlock are already being done.
Dependency vdd handing is not required in this case and if I use goto,
then I cannot skip dependency handling since it is done after unlocking
mutex.
>
> >  			mutex_unlock(&dvfs_info->scaling_mutex);
> >  			return ret;
> >  		}
> > @@ -570,6 +574,9 @@ static int omap_dvfs_voltage_scale(struct
> omap_vdd_dvfs_info *dvfs_info)
> >  	if (!is_volt_scaled && !ret)
> >  		omap_voltage_scale_vdd(voltdm, volt);
> >
> > +	/* Enable Smartreflex module */
>
> comment redundant
ok

Vishwa
>
> > +	omap_sr_enable(voltdm);
> > +
> >  	mutex_unlock(&dvfs_info->scaling_mutex);
> >
> >  	/* calculate the voltages for dependent vdd's */
>
> Kevin

RE: [PATCH 01/13] OMAP: Introduce accessory APIs for DVFS

[Vishwanath Sripathy]

> -----Original Message-----
> From: Kevin Hilman [mailto:khilman@ti.com]
> Sent: Thursday, February 03, 2011 6:37 AM
> To: Vishwanath BS
> Cc: linux-omap@vger.kernel.org; patches@linaro.org; Thara Gopinath
> Subject: Re: [PATCH 01/13] OMAP: Introduce accessory APIs for DVFS
>
> Vishwanath BS <vishwanath.bs@ti.com> writes:
>
> > This patch introduces accessory APIs for DVFS.
>
> Actually, it begins the implementation of a DVFS layer.
>
> > Data structures added:
> > 1. omap_dev_user_list: This structure maintains list of frequency
> requests per
> >    device basis. When a device needs to be scaled to a particular
> frequency,
> >    This list is searched to find the maximum request for a given
device.
> >    If noone has placed any request, device frequency is obtained from
> device
> >    opp table.
> > 2. omap_vdd_dev_list: This strcucture stores device list per vdd
basis.
> >    Whenever a device is registered with a vdd, it is added to this
list.
> > 3. omap_vdd_user_list: 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.
> > 4. omap_vdd_dvfs_info: This structure is used to abstract DVFS
> related
> >    information per VDD basis. It holds pointer to corresponding vdd's
> >    voltagedomain instance and pointer to user list.
>
> The terms "user" and dev/device are rather overloaded here and
> elsewhere
> in the code, and makes for rather difficult reading.
>
> I have an idea (or two) to rework these data structures and how they're
> stored/connected, but I need to think on it a little more.  More on that
> tomorrow...
>
> > Following APIs have been added to operate on above data structures:
> > 1. omap_dvfs_add_vdd_user - API to add a user into
> omap_vdd_user_list
> > 2. omap_vdd_user_list - API to remove a user from
> omap_vdd_user_list
>
> huh?  cut and paste error?  I think this was supposed to be
> _remove_vdd_user() ?
oops..sorry for the typo. Will fix it.
>
> > 3. omap_dvfs_register_device - API to register a device with vdd
> > 4. omap_dvfs_add_freq_request - API to add a frequency request into
> >    omap_dev_user_list
> > 5. omap_dvfs_remove_freq_request - API to remove a frequency
> request from
> >    omap_dev_user_list
> > 6. omap_dvfs_find_voltage - API to find the opp corresponding to
> given voltage
>
> I think function naming needs rework for consistency.  For example, to
> request a frequency you call _add_freq_request(), but to add a voltage
> you call _add_vdd_user(), not very reader friendly
>
> How about simply:
>
>     omap_dvfs_request_freq()
>     omap_dvfs_request_volt()
>
> with remove equivalents.
>
> Actually, looking more at the functions in this patch, the frequency and
> voltage functions here are basically identical.  There really isn't any
> good reason to have separate functions for frequency and voltage.  How
> about:
>
>     omap_dvfs_add_request(dvfs_info, class, req_dev, target_dev);
>     omap_dvfs_remove_request(dvfs_info, class, req_dev, target_dev);
>
> where class = OMAP_DVFS_FREQ | OMAP_DVFS_VOLT.  This way, based
> on the
> class, you can add the requests to the separate lists, but bulk
> of the function is the same.
I am not sure if this is a good idea to club them into the same function.
If you look at those functions, common thing done by them is adding to the
list. But actually the way the list is found and the actual list being
added are completely different.
Voltage request is added to the user list per VDD.
Frequency request is added to user list per device and we have device list
per vdd. That is the reason why target_dev is needed while adding
frequency request and is not needed while adding voltage request.
I feel clubbing these 2 functions will make function bulky and more
unreadable. I can rename these 2 functions like the way you suggested.
>
> >
> > DVFS layer is initialized and basic data structures are allocated and
> > initialized as part of this.
> >
> > This patch is based on Thara's previous DVFS implementation, but with
> major
> > rework.
> >
>
> Minor comment on acronyms.   In the comments throughout, please
> capitalize acronyms like DVFS, VDD, etc.  Currently, it is mixed between
> upper and lower-case.
OK. Will fix it in next version.
>
> > Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
> > Cc: Thara Gopinath <thara@ti.com>
> > ---
> >  arch/arm/mach-omap2/Makefile           |    2 +-
> >  arch/arm/mach-omap2/dvfs.c             |  456
> ++++++++++++++++++++++++++++++++
> >  arch/arm/plat-omap/include/plat/dvfs.h |   27 ++
> >  arch/arm/plat-omap/omap_device.c       |    9 +
> >  4 files changed, 493 insertions(+), 1 deletions(-)
> >  create mode 100644 arch/arm/mach-omap2/dvfs.c
> >  create mode 100644 arch/arm/plat-omap/include/plat/dvfs.h
> >
> > diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-
> omap2/Makefile
> > index 4ab82f6..bb2e2bc 100644
> > --- a/arch/arm/mach-omap2/Makefile
> > +++ b/arch/arm/mach-omap2/Makefile
> > @@ -61,7 +61,7 @@ ifeq ($(CONFIG_PM),y)
> >  obj-$(CONFIG_ARCH_OMAP2)		+= pm24xx.o
> >  obj-$(CONFIG_ARCH_OMAP2)		+= sleep24xx.o pm_bus.o
> voltage.o
> >  obj-$(CONFIG_ARCH_OMAP3)		+= pm34xx.o sleep34xx.o
> voltage.o \
> > -					   cpuidle34xx.o pm_bus.o
> > +					   cpuidle34xx.o pm_bus.o dvfs.o
> >  obj-$(CONFIG_ARCH_OMAP4)		+= pm44xx.o voltage.o
> pm_bus.o
> >  obj-$(CONFIG_PM_DEBUG)			+= pm-debug.o
> >  obj-$(CONFIG_OMAP_SMARTREFLEX)          += sr_device.o
> smartreflex.o
> > diff --git a/arch/arm/mach-omap2/dvfs.c b/arch/arm/mach-
> omap2/dvfs.c
> > new file mode 100644
> > index 0000000..8832e4a
> > --- /dev/null
> > +++ b/arch/arm/mach-omap2/dvfs.c
> > @@ -0,0 +1,456 @@
> > +/*
> > + * OMAP3/OMAP4 DVFS Management Routines
> > + *
> > + * Author: Vishwanath BS	<vishwanath.bs@ti.com>
> > + *
> > + * Copyright (C) 2011 Texas Instruments, Inc.
> > + * Vishwanath BS <vishwanath.bs@ti.com>
> > + *
> > + * This program is free software; you can redistribute it and/or
> modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +
> > +#include <linux/err.h>
> > +#include <linux/spinlock.h>
> > +#include <linux/plist.h>
> > +#include <linux/slab.h>
> > +#include <linux/opp.h>
> > +#include <plat/common.h>
> > +#include <plat/voltage.h>
> > +#include <plat/omap_device.h>
> > +
> > +/**
> > + * struct omap_dev_user_list - Structure maitain userlist per devide
>
> typos: maintain, device
>
> the latter typo you fix in a follow up patch, but should be fixed here.
ok

>
> > + *
> > + * @dev:       The device requesting for a particular frequency
> > + * @node:      The list head entry
> > + * @freq:      frequency being requested
> > + *
> > + * Using this structure, user list (requesting dev * and frequency)
for
> > + * each device is maintained. This is how we can have different
> devices
> > + * at different frequencies (to support frequency locking and
> throttling).
> > + * Even if one of the devices in a given vdd has locked it's
frequency,
> > + * other's can still scale their frequency using this list.
> > + * If no one has placed a frequency request for a device, then device
> is
> > + * set to the frequency from it's opp table.
> > + */
> > +struct omap_dev_user_list {
> > +		struct device *dev;
> > +		struct plist_node node;
> > +		u32 freq;
>
> extra indentation
ok
>
> > +};
> > +
> > +/**
> > + * struct omap_vdd_dev_list - Device list per vdd
> > + *
> > + * @dev:	The device belonging to a particular vdd
> > + * @node:	The list head entry
>
> and the other entries?
Will fix it
>
> > + */
> > +struct omap_vdd_dev_list {
> > +	struct device *dev;
> > +	struct list_head node;
> > +	struct plist_head user_list;
> > +	spinlock_t user_lock; /* spinlock for plist */
>
> comment redundant
ok
>
> > +};
> > +
> > +/**
> > + * struct 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;
> > +};
>
> This struct is identical to omap_dev_user_list above, except
> s/freq/volt/.  You probably just need a single struct using 'val'
> instead of freq/volt.
Make sense. I will keep a single struct as you suggested.
>
> That being said, why is the frequency struct called 'dev_user_list' and
> the voltage table called 'vdd_user_list'.  This alone renders the
> resulting code unreadable.
dev_user_list - device user list, referring to frequency requests related
to the device.
vdd_user_list - voltage requests related to the given vdd.
I will club these 2 and create a single structure with the name
omap_opp_req_list.
>
>
> > +/**
> > + * struct omap_vdd_dvfs_info - The per vdd dvfs info
> > + *
> > + * @user_lock:	spinlock for plist operations
> > + * @user_list:	The vdd user list
> > + * @scaling_mutex:	Mutex for protecting dvfs data structures for
> a vdd
> > + * @voltdm: Voltage domains for which dvfs info stored
>
> missing some entries here
ok. Will fix it.
>
> > + * This is a fundamental structure used to store all the required
> > + * DVFS related information for a vdd.
> > + */
> > +struct omap_vdd_dvfs_info {
> > +	spinlock_t user_lock; /* spin lock */
>
> comment redundant
I added this because checkpatch was giving a warning saying mutex added
w/o comments.>
> > +	struct plist_head user_list;
> > +	struct mutex scaling_mutex; /* dvfs mutex */
>
> ditto
Same reason.
>
> > +	struct voltagedomain *voltdm;
> > +	struct list_head dev_list;
> > +};
> > +
> > +static struct omap_vdd_dvfs_info *omap_dvfs_info_list;
> > +static int omap_nr_vdd;
> > +
> > +static struct voltagedomain omap3_vdd[] = {
> > +	{
> > +	.name = "mpu",
> > +	},
> > +	{
> > +	.name = "core",
> > +	},
> > +};
>
> indentation
ok
>
> > +static int __init omap_dvfs_init(void);
> > +
> > +static struct omap_vdd_dvfs_info *get_dvfs_info(struct
> voltagedomain *voltdm)
> > +{
> > +	int i;
>
> insert blank line
ok
>
> > +	if (!voltdm || !omap_dvfs_info_list)
> > +		return NULL;
> > +
> > +	for (i = 0; i < omap_nr_vdd; i++)
> > +		if (omap_dvfs_info_list[i].voltdm == voltdm)
> > +			return &omap_dvfs_info_list[i];
> > +
> > +	pr_warning("%s: unable find dvfs info for vdd %s\n",
> > +			__func__, voltdm->name);
> > +	return NULL;
> > +}
> > +
> > +/**
> > + * omap_dvfs_find_voltage() - search for given 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
>
> comment doesn't match code.  OPP lookup is done using the 'ceil'
> version, which is not an exact match, and the voltage check uses '>='
> not '=='.
Ok. Will fix the comment.
>
> > + * opp if found, else returns ERR_PTR in case of error and should be
> handled
> > + * using IS_ERR. If there are multiple opps with same voltage, it
will
> return
> > + * the first available entry.
>
> More specifically, it will return the one with the lowest frequency.
Yes.
>
> > + */
> > +static struct opp *omap_dvfs_find_voltage(struct device *dev,
> > +		unsigned long volt)
> > +{
> > +	struct opp *opp = ERR_PTR(-ENODEV);
> > +	unsigned long f = 0;
> > +
> > +	do {
> > +		opp = opp_find_freq_ceil(dev, &f);
> > +		if (IS_ERR(opp))
> > +			break;
> > +		if (opp_get_voltage(opp) >= volt)
> > +			break;
> > +		f++;
> > +	} while (1);
> > +
> > +	return opp;
> > +}
> > +
> > +/**
> > + * omap_dvfs_add_vdd_user() - Add a voltage request
> > + * @dvfs_info: omap_vdd_dvfs_info pointer for the required vdd
> > + * @dev: device making the request
> > + * @volt: requesting voltage in uV
> > + *
> > + * Adds the given devices' voltage request into corresponding
> > + * vdd's omap_vdd_dvfs_info user list (plist). This list is used
> > + * to find the maximum voltage request for a given vdd.
> > + *
> > + * Returns 0 on success.
> > + */
> > +static int omap_dvfs_add_vdd_user(struct omap_vdd_dvfs_info
> *dvfs_info,
> > +		struct device *dev, unsigned long volt)
> > +{
> > +	struct omap_vdd_user_list *user = NULL, *temp_user;
> > +	struct plist_node *node;
> > +
> > +	if (!dvfs_info || IS_ERR(dvfs_info)) {
> > +		dev_warn(dev, "%s: VDD specified does not exist!\n",
> __func__);
> > +		return -EINVAL;
> > +	}
> > +
> > +	mutex_lock(&dvfs_info->scaling_mutex);
> > +
> > +	plist_for_each_entry(temp_user, &dvfs_info->user_list, node) {
> > +		if (temp_user->dev == dev) {
> > +			user = temp_user;
> > +			break;
> > +		}
> > +	}
> > +
> > +	if (!user) {
> > +		user = kzalloc(sizeof(struct omap_vdd_user_list),
> GFP_KERNEL);
> > +		if (!user) {
> > +			dev_err(dev, "%s: Unable to creat a new user for
> vdd_%s\n",
> > +				__func__, dvfs_info->voltdm->name);
> > +			mutex_unlock(&dvfs_info->scaling_mutex);
> > +			return -ENOMEM;
> > +		}
> > +		user->dev = dev;
> > +	} else {
> > +		plist_del(&user->node, &dvfs_info->user_list);
> > +	}
> > +
> > +	plist_node_init(&user->node, volt);
> > +	plist_add(&user->node, &dvfs_info->user_list);
> > +	node = plist_last(&dvfs_info->user_list);
> > +	user->volt = node->prio;
> > +
> > +	mutex_unlock(&dvfs_info->scaling_mutex);
> > +
> > +	return 0;
> > +}
> > +
> > +/**
> > + * omap_dvfs_remove_vdd_user() - Remove a voltage request
> > + * @dvfs_info: omap_vdd_dvfs_info pointer for the required vdd
> > + * @dev: device making the request
> > + *
> > + * Removes the given devices' voltage request from corresponding
> > + * vdd's omap_vdd_dvfs_info user list (plist).
> > + *
> > + * Returns 0 on success.
> > + */
> > +static int omap_dvfs_remove_vdd_user(struct omap_vdd_dvfs_info
> *dvfs_info,
> > +		struct device *dev)
> > +{
> > +	struct omap_vdd_user_list *user = NULL, *temp_user;
> > +	int ret = 0;
> > +
> > +	if (!dvfs_info || IS_ERR(dvfs_info)) {
> > +		dev_err(dev, "%s: VDD specified does not exist!\n",
> __func__);
> > +		return -EINVAL;
> > +	}
> > +
> > +	mutex_lock(&dvfs_info->scaling_mutex);
> > +
> > +	plist_for_each_entry(temp_user, &dvfs_info->user_list, node) {
> > +		if (temp_user->dev == dev) {
> > +			user = temp_user;
> > +			break;
> > +		}
> > +	}
> > +
> > +	if (user)
> > +		plist_del(&user->node, &dvfs_info->user_list);
> > +	else {
> > +		dev_err(dev, "%s: Unable to find the user for vdd_%s\n",
> > +					__func__, dvfs_info->voltdm-
> >name);
> > +		ret = -ENOMEM;
> > +	}
> > +	mutex_unlock(&dvfs_info->scaling_mutex);
> > +
> > +	return ret;
> > +}
> > +
> > +/**
> > + * omap_dvfs_register_device - Add a device into voltage domain
> > + * @voltdm:	voltage domain to which the device is to be added
> > + * @dev:	Device to be added
> > + *
> > + * This API will add a given device into user_list of corresponding
> > + * vdd's omap_vdd_dvfs_info strucure. This list is traversed to scale
> > + * frequencies of all the devices on a given vdd. This api is called
> > + * while hwmod db is built for an omap_device.
> > + *
> > + * Returns 0 on success.
> > + */
> > +int omap_dvfs_register_device(struct voltagedomain *voltdm, struct
> device *dev)
> > +{
> > +	struct omap_vdd_dev_list *temp_dev;
>
> The point of using the 'temp_' names is for temporary iterators.  Here,
> you're using it as the dev_list throughout.  Just call it dev_list.
ok
>
> > +	struct omap_vdd_dvfs_info *dvfs_info = get_dvfs_info(voltdm);
> > +
> > +	if (!voltdm || IS_ERR(voltdm) || !dvfs_info) {
> > +		dev_warn(dev, "%s: VDD specified does not exist!\n",
> __func__);
> > +		return -EINVAL;
> > +	}
> > +
> > +	list_for_each_entry(temp_dev, &dvfs_info->dev_list, node) {
> > +		if (temp_dev->dev == dev) {
> > +			dev_warn(dev, "%s: Device already added to
> vdee_%s\n",
>
> s/vdee/VDD/ ?
>
> You might be seeing device already added for devices with multiple
> hwmods per omap_device.  More on this below.
>
> > +				__func__, dvfs_info->voltdm->name);
> > +			return -EINVAL;
> > +		}
> > +	}
> > +
> > +	temp_dev = kzalloc(sizeof(struct omap_vdd_dev_list),
> GFP_KERNEL);
> > +	if (!temp_dev) {
> > +		dev_err(dev, "%s: Unable to creat a new device for
> vdd_%s\n",
> > +			__func__, dvfs_info->voltdm->name);
> > +		return -ENOMEM;
> > +	}
> > +
> > +	/* Initialize priority ordered list */
> > +	spin_lock_init(&temp_dev->user_lock);
> > +	plist_head_init(&temp_dev->user_list, &temp_dev->user_lock);
> > +
> > +	temp_dev->dev = dev;
> > +	list_add(&temp_dev->node, &dvfs_info->dev_list);
> > +
> > +	return 0;
> > +}
> > +
> > +/**
> > + * omap_dvfs_add_freq_request() - add a requested device
> frequency
> > + *
> > + *
> > + * @dvfs_info: omap_vdd_dvfs_info pointer for the required vdd
> > + * @req_dev: device making the request
> > + * @target_dev: target device for which frequency request is being
> made
> > + * @freq:	target device frequency
> > + *
> > + * This API adds a requested frequency into target's device frequency
> list.
>
> more documentation needed for the reasoning behind why both a
> requesting
> device and a target device are needed.  Also, whey they are needed for
> frequency and not voltage.  That might affect my idea above about
> having
> the same function for adding freq/voltage requests.  Even so, the core
> part of these functions is identical, so some common functions should
> probably be used.
I already addressed this in one of the previous comments. Will add more
comments in function comments.
>
> > + * Returns 0 on success.
> > + */
> > +static int omap_dvfs_add_freq_request(struct omap_vdd_dvfs_info
> *dvfs_info,
> > +	struct device *req_dev, struct device *target_dev, unsigned long
> freq)
> > +{
> > +	struct omap_dev_user_list *dev_user = NULL, *tmp_user;
> > +	struct omap_vdd_dev_list *temp_dev;
>
> 	struct omap_vdd_dev_list *dev_list;
>
> > +	if (!dvfs_info || IS_ERR(dvfs_info)) {
> > +		dev_warn(target_dev, "%s: VDD specified does not
> exist!\n",
> > +			__func__);
> > +		return -EINVAL;
> > +	}
> > +
> > +	mutex_lock(&dvfs_info->scaling_mutex);
> > +
> > +	list_for_each_entry(temp_dev, &dvfs_info->dev_list, node) {
> > +		if (temp_dev->dev == target_dev)
>
> 			dev_list = temp_dev;
>
> > +			break;
> > +	}
> > +	if (temp_dev->dev != target_dev) {
>
> 	if (!dev_list) {
>
> > +		dev_warn(target_dev, "%s: target_dev does not exist!\n",
> > +			__func__);
> > +		mutex_unlock(&dvfs_info->scaling_mutex);
> > +		return -EINVAL;
> > +	}
> > +
> > +	plist_for_each_entry(tmp_user, &temp_dev->user_list, node) {
> > +		if (tmp_user->dev == req_dev) {
> > +			dev_user = tmp_user;
> > +			break;
> > +		}
> > +	}
> > +
> > +	if (!dev_user) {
> > +		dev_user = kzalloc(sizeof(struct omap_dev_user_list),
> > +					GFP_KERNEL);
> > +		if (!dev_user) {
> > +			dev_err(target_dev, "%s: Unable to creat a new
> user for vdd_%s\n",
> > +				__func__, dvfs_info->voltdm->name);
> > +			mutex_unlock(&dvfs_info->scaling_mutex);
> > +			return -ENOMEM;
> > +		}
> > +		dev_user->dev = req_dev;
> > +	} else {
> > +		plist_del(&dev_user->node, &temp_dev->user_list);
> > +	}
> > +
> > +	plist_node_init(&dev_user->node, freq);
> > +	plist_add(&dev_user->node, &temp_dev->user_list);
> > +
> > +	mutex_unlock(&dvfs_info->scaling_mutex);
> > +	return 0;
> > +}
> > +
> > +/**
> > + * omap_dvfs_remove_freq_request() - Remove the requested
> device frequency
> > + *
> > + * @dvfs_info: omap_vdd_dvfs_info pointer for the required vdd
> > + * @req_dev: device removing the request
> > + * @target_dev: target device from which frequency request is being
> removed
> > + *
> > + * This API removes a requested frequency from target's device
> frequency list.
> > + *
> > + * Returns 0 on success.
> > + */
> > +
> > +static int omap_dvfs_remove_freq_request(struct
> omap_vdd_dvfs_info *dvfs_info,
> > +	struct device *req_dev, struct device *target_dev)
> > +{
> > +	struct omap_dev_user_list *dev_user = NULL, *tmp_user;
> > +	int ret = 0;
> > +	struct omap_vdd_dev_list *temp_dev;
> > +
> > +	if (!dvfs_info || IS_ERR(dvfs_info)) {
> > +		dev_warn(target_dev, "%s: VDD specified does not
> exist!\n",
> > +			__func__);
> > +		return -EINVAL;
> > +	}
> > +
> > +	mutex_lock(&dvfs_info->scaling_mutex);
> > +
> > +	list_for_each_entry(temp_dev, &dvfs_info->dev_list, node) {
> > +		if (temp_dev->dev == target_dev)
> > +			break;
> > +	}
> > +
> > +	if (temp_dev->dev != target_dev) {
> > +		dev_warn(target_dev, "%s: target_dev does not exist!\n",
> > +			__func__);
> > +		mutex_unlock(&dvfs_info->scaling_mutex);
> > +		return -EINVAL;
> > +	}
> > +
> > +	plist_for_each_entry(tmp_user, &temp_dev->user_list, node) {
> > +		if (tmp_user->dev == req_dev) {
> > +			dev_user = tmp_user;
> > +			break;
> > +		}
> > +	}
> > +
> > +	if (dev_user)
> > +		plist_del(&dev_user->node, &temp_dev->user_list);
> > +	else {
> > +		dev_err(target_dev, "%s: Unable to remove the user for
> vdd_%s\n",
> > +				__func__, dvfs_info->voltdm->name);
> > +			ret = -EINVAL;
> > +		}
> > +
> > +	return ret;
> > +}
> > +
> > +/**
> > + * omap_dvfs_init() - Initialize omap dvfs layer
> > + *
> > + * Initalizes omap dvfs layer. It basically allocates memory for
> > + * omap_dvfs_info_list and  populates voltdm pointer inside
> > + * omap_vdd_dvfs_info structure for all the VDDs.
> > + *
> > + * Returns 0 on success.
> > + */
> > +static int __init omap_dvfs_init()
> > +{
> > +	int i;
> > +	struct voltagedomain *vdd_list;
>
> insert blank line
ok
>
> > +	if (cpu_is_omap34xx()) {
> > +		omap_nr_vdd = 2;
>
> use ARRAY_SIZE(omap3_vdd)
ok
>
> > +		vdd_list = omap3_vdd;
> > +	}
>
> else?
Should throw error and return. Will fix it.
>
> > +	omap_dvfs_info_list = kzalloc(omap_nr_vdd *
> > +			sizeof(struct omap_vdd_dvfs_info), GFP_KERNEL);
> > +	if (!omap_dvfs_info_list) {
> > +		pr_warning("%s: Unable to allocate memory for vdd_list",
> > +			__func__);
> > +		return -ENOMEM;
> > +	}
> > +
> > +	for (i = 0; i < omap_nr_vdd; i++) {
> > +		omap_dvfs_info_list[i].voltdm =
> > +			omap_voltage_domain_lookup(vdd_list[i].name);
> > +		/* Init the plist */
>
> comments redundant
>
> > +		spin_lock_init(&omap_dvfs_info_list[i].user_lock);
> > +		plist_head_init(&omap_dvfs_info_list[i].user_list,
> > +
&omap_dvfs_info_list[i].user_lock);
>
> ditto
>
> > +		/* Init the DVFS mutex */
> > +		mutex_init(&omap_dvfs_info_list[i].scaling_mutex);
> > +		/* Init the device list */
>
> ditto
ok
>
> > +		INIT_LIST_HEAD(&omap_dvfs_info_list[i].dev_list);
>
>
> > +	}
> > +
> > +	return 0;
> > +}
> > +core_initcall(omap_dvfs_init);
> > diff --git a/arch/arm/plat-omap/include/plat/dvfs.h b/arch/arm/plat-
> omap/include/plat/dvfs.h
> > new file mode 100644
> > index 0000000..1302990
> > --- /dev/null
> > +++ b/arch/arm/plat-omap/include/plat/dvfs.h
> > @@ -0,0 +1,27 @@
> > +/*
> > + * OMAP3/OMAP4 DVFS Management Routines
> > + *
> > + * Author: Vishwanath BS	<vishwanath.bs@ti.com>
> > + *
> > + * Copyright (C) 2011 Texas Instruments, Inc.
> > + * Vishwanath BS <vishwanath.bs@ti.com>
> > + *
> > + * This program is free software; you can redistribute it and/or
> modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +
> > +#ifndef __ARCH_ARM_MACH_OMAP2_DVFS_H
> > +#define __ARCH_ARM_MACH_OMAP2_DVFS_H
> > +#include <plat/voltage.h>
> > +
> > +#ifdef CONFIG_PM
> > +int omap_dvfs_register_device(struct voltagedomain *voltdm, struct
> device *dev);
> > +#else
> > +static inline int omap_dvfs_register_device(struct voltagedomain
> *voltdm,
> > +		struct device *dev)
> > +{
> > +	return -EINVAL;
> > +}
> > +#endif
> > +#endif
> > diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-
> omap/omap_device.c
> > index 57adb27..a84e849 100644
> > --- a/arch/arm/plat-omap/omap_device.c
> > +++ b/arch/arm/plat-omap/omap_device.c
> > @@ -86,6 +86,7 @@
> >
> >  #include <plat/omap_device.h>
> >  #include <plat/omap_hwmod.h>
> > +#include <plat/dvfs.h>
> >
> >  /* These parameters are passed to _omap_device_{de,}activate() */
> >  #define USE_WAKEUP_LAT			0
> > @@ -481,6 +482,14 @@ struct omap_device
> *omap_device_build_ss(const char *pdev_name, int pdev_id,
> >  	for (i = 0; i < oh_cnt; i++) {
> >  		hwmods[i]->od = od;
> >  		_add_optional_clock_alias(od, hwmods[i]);
> > +		if (!is_early_device && hwmods[i]->vdd_name) {
> > +			struct omap_hwmod *oh = hwmods[i];
> > +			struct voltagedomain *voltdm;
> > +
> > +			voltdm = omap_voltage_domain_lookup(oh-
> >vdd_name);
> > +			if (!omap_dvfs_register_device(voltdm, &od-
> >pdev.dev))
> > +				oh->voltdm = voltdm;
> > +		}
>
> This should be taken out of the for loop.  Otherwise, if an omap_device
> has multiple hwmods, then the same devices is registered with DVFS
> twice.
OK understood. Will fix it.

Vishwa
>
> >  	}
> >
> >  	if (ret)
>
> Kevin

Re: [PATCH 01/13] OMAP: Introduce accessory APIs for DVFS

[Kevin Hilman]

Vishwanath Sripathy <vishwanath.bs@ti.com> writes:

[...]

>> > + * This is a fundamental structure used to store all the required
>> > + * DVFS related information for a vdd.
>> > + */
>> > +struct omap_vdd_dvfs_info {
>> > +	spinlock_t user_lock; /* spin lock */
>>
>> comment redundant
>>
>
> I added this because checkpatch was giving a warning saying mutex added
> w/o comments.
>

The point of the checkpatch warning is not to add just any comment.  The
point is to add a *useful* comment.

It is extremely helpful to readers of code with locking to know what the
locks are intended to protect.  The comment should indicate what the
lock is protecting and why.

Kevin

Re: [PATCH 03/13] OMAP: Implement Basic DVFS

[Kevin Hilman]

Vishwanath Sripathy <vishwanath.bs@ti.com> writes:

>> This needs a comment to, but I'm not sure I understand what's going on
>> here.  What it seems like:
>>
>> if this device has no OPP for this voltage, just silently move on to the
>> next device?   doesn't seem quite right, but not sure I fully grok the
>> failure modes of omap_dvfs_find_voltage()
>
> Yes, your understanding is right. omap_dvfs_find_voltage will return error
> if the device does not have an OPP table.
> Typically devices should not register with a vdd (using
> omap_dvfs_register_device) if it has no OPP table associated with it. So
> ideally we should not hit this error case. But only exception so far is SR
> driver. SR hwmod has vdd_name field set so as to get voltagedomain
> pointers. But SR does not have any opp table. So there is no harm in
> ignoring the error and moving to next device.

And what happens when other devices add voltage domains but don't have
OPP tables?

The point is that this error handling is 1) difficult to understand upon
first (or fifth) read and 2) very fragile with other changes.

Kevin

RE: [PATCH 03/13] OMAP: Implement Basic DVFS

[Vishwanath Sripathy]

> -----Original Message-----
> From: Kevin Hilman [mailto:khilman@ti.com]
> Sent: Wednesday, February 09, 2011 9:30 PM
> To: Vishwanath Sripathy
> Cc: linux-omap@vger.kernel.org; patches@linaro.org; Thara Gopinath
> Subject: Re: [PATCH 03/13] OMAP: Implement Basic DVFS
>
> Vishwanath Sripathy <vishwanath.bs@ti.com> writes:
>
> >> This needs a comment to, but I'm not sure I understand what's going
> on
> >> here.  What it seems like:
> >>
> >> if this device has no OPP for this voltage, just silently move on to
the
> >> next device?   doesn't seem quite right, but not sure I fully grok
the
> >> failure modes of omap_dvfs_find_voltage()
> >
> > Yes, your understanding is right. omap_dvfs_find_voltage will return
> error
> > if the device does not have an OPP table.
> > Typically devices should not register with a vdd (using
> > omap_dvfs_register_device) if it has no OPP table associated with it.
> So
> > ideally we should not hit this error case. But only exception so far
is SR
> > driver. SR hwmod has vdd_name field set so as to get voltagedomain
> > pointers. But SR does not have any opp table. So there is no harm in
> > ignoring the error and moving to next device.
>
> And what happens when other devices add voltage domains but don't
> have
> OPP tables?
If someone does not have a OPP table, that means it's not a scalable
device, so there is no need to scale that device.

Vishwa
>
> The point is that this error handling is 1) difficult to understand upon
> first (or fifth) read and 2) very fragile with other changes.
>
> Kevin

Re: [PATCH 04/13] OMAP: Introduce dependent voltage domain support

[Kevin Hilman]

Vishwanath Sripathy <vishwanath.bs@ti.com> writes:

[...]

>> > diff --git a/arch/arm/mach-omap2/dvfs.c b/arch/arm/mach-
>> omap2/dvfs.c
>> > index cefc2be..c9d3894 100755
>> > --- a/arch/arm/mach-omap2/dvfs.c
>> > +++ b/arch/arm/mach-omap2/dvfs.c
>> > @@ -85,6 +85,7 @@ struct omap_vdd_dvfs_info {
>> >  	struct mutex scaling_mutex; /* dvfs mutex */
>> >  	struct voltagedomain *voltdm;
>> >  	struct list_head dev_list;
>> > +	struct device vdd_device;
>>
>> It's not clear what the usage of this device is for.
>>
>> It is never initialized, but seems to be used as a dummy device when
>> calcluating dependencies.   Needs clarification.
>
> This device is used for placing voltage request (omap_dvfs_add_vdd_user)
> when dealing with dependent vdds. Eg: while scaling MPU VDD, we also scale
> CORE VDD. So while placing voltage request for CORE VDD, this vdd_device
> of MPU is used as the requesting device so that this request is stored as
> a separate user.

I was able to follow how it was used, and why.   But it is still not
clear to the reader.

First, the device is never initialized, so it's essentially a dummy
device that remains initialized by default to all zeros.   So, any
attempt to use any device APIs (e.g. dev_name, etc.) on this will not
behave as expected.

Second, the reason for this device and the need for the request to be
stored as a separate user are not clear in the code and don't exist in
the comments.

Kevin

Re: [PATCH 05/13] OMAP: Introduce device scale implementation

[Kevin Hilman]

Vishwanath Sripathy <vishwanath.bs@ti.com> writes:

>> -----Original Message-----
>> From: Kevin Hilman [mailto:khilman@ti.com]
>> Sent: Friday, February 04, 2011 9:35 PM
>> To: Vishwanath BS
>> Cc: linux-omap@vger.kernel.org; patches@linaro.org; Thara Gopinath
>> Subject: Re: [PATCH 05/13] OMAP: Introduce device scale
>> implementation
>>
>> Vishwanath BS <vishwanath.bs@ti.com> writes:
>>
>> > This patch adds omap_device_scale API  which can be used to generic
>> > device rate scaling.
>>
>> I would've expected a new omap_device_* API to be part of the
>> omap_device layer, not added here.
> Given that this API does not deal with any of the omap_device layer
> functions or data structures, I am not sure if it logically belongs to
> omap device layer. If you think it should belong to omap_device layer just
> because the name starts with omap_device, I would rather rename this API.

That's up to you.  

But if it's not an omap_device API, then it shouldn't be named
omap_device_*

Kevin

>>
>> > Based on original patch from Thara.
>> >
>> > Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
>> > Cc: Thara Gopinath <thara@ti.com>
>> > ---
>> >  arch/arm/mach-omap2/dvfs.c             |  116
>> ++++++++++++++++++++++++++++++++
>> >  arch/arm/plat-omap/include/plat/dvfs.h |    7 ++
>> >  2 files changed, 123 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/arch/arm/mach-omap2/dvfs.c b/arch/arm/mach-
>> omap2/dvfs.c
>> > index c9d3894..05a9ce3 100755
>> > --- a/arch/arm/mach-omap2/dvfs.c
>> > +++ b/arch/arm/mach-omap2/dvfs.c
>> > @@ -19,6 +19,7 @@
>> >  #include <plat/common.h>
>> >  #include <plat/voltage.h>
>> >  #include <plat/omap_device.h>
>> > +#include <plat/smartreflex.h>
>> >
>> >  /**
>> >   * struct omap_dev_user_list - Structure maitain userlist per device
>> > @@ -585,6 +586,121 @@ static int omap_dvfs_voltage_scale(struct
>> omap_vdd_dvfs_info *dvfs_info)
>> >  }
>> >
>> >  /**
>> > + * omap_device_scale() - Set a new rate at which the device is to
>> operate
>> > + * @req_dev:	pointer to the device requesting the scaling.
>> > + * @target_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
>> > + */
>>
>> Here would be a good place to describe why both the requesting device
>> and the target device need to be tracked.
> OK
>
>>
>> > +int omap_device_scale(struct device *req_dev, struct device
>> *target_dev,
>> > +			unsigned long rate)
>> > +{
>> > +	struct opp *opp;
>> > +	unsigned long volt, freq, min_freq, max_freq;
>> > +	struct omap_vdd_dvfs_info *dvfs_info;
>> > +	struct platform_device *pdev;
>> > +	struct omap_device *od;
>> > +	int ret = 0;
>> > +
>> > +	pdev = container_of(target_dev, struct platform_device, dev);
>> > +	od = container_of(pdev, struct omap_device, pdev);
>> > +
>> > +	/*
>> > +	 * Figure out if the desired frquency lies between the
>> > +	 * maximum and minimum possible for the particular device
>> > +	 */
>> > +	min_freq = 0;
>> > +	if (IS_ERR(opp_find_freq_ceil(target_dev, &min_freq))) {
>> > +		dev_err(target_dev, "%s: Unable to find lowest opp\n",
>> > +						__func__);
>> > +		return -ENODEV;
>> > +	}
>> > +
>> > +	max_freq = ULONG_MAX;
>> > +	if (IS_ERR(opp_find_freq_floor(target_dev, &max_freq))) {
>> > +		dev_err(target_dev, "%s: Unable to find highest opp\n",
>> > +						__func__);
>> > +		return -ENODEV;
>> > +	}
>> > +
>> > +	if (rate < min_freq)
>> > +		freq = min_freq;
>> > +	else if (rate > max_freq)
>> > +		freq = max_freq;
>> > +	else
>> > +		freq = rate;
>> > +
>>
>> OK, frome here on, I would expect the adjusted value 'freq' to be used
>> instead of 'rate', but that is not the case below.
>>
>> > +	opp = opp_find_freq_ceil(target_dev, &freq);
>> > +	if (IS_ERR(opp)) {
>> > +		dev_err(target_dev, "%s: Unable to find OPP for
>> freq%ld\n",
>> > +			__func__, rate);
>> > +		return -ENODEV;
>> > +	}
> Freq is appropriate than rate here. Will fix it.
>> > +
>> > +	/* 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.
>> > +	 */
>>
>> This comment doesn't match the following code.
> OK. Will fix it. Copy paste error.
>>
>> > +	if (rate) {
>>
>> Why is rate used here, and not freq?
> Freq can never be 0. If somebody wants to remove his DVFS request (he does
> not really care about the device frequency), then he can pass rate as 0.
> Hence rate is used.
>>
>> > +		dvfs_info = get_dvfs_info(od->hwmods[0]->voltdm);
>> > +
>> > +		ret = omap_dvfs_add_freq_request(dvfs_info, req_dev,
>> > +						target_dev, freq);
>> > +		if (ret) {
>> > +			dev_err(target_dev, "%s: Unable to add frequency
>> request\n",
>> > +				__func__);
>> > +			return ret;
>> > +		}
>> > +
>> > +		ret = omap_dvfs_add_vdd_user(dvfs_info, req_dev, volt);
>> > +		if (ret) {
>> > +			dev_err(target_dev, "%s: Unable to add voltage
>> request\n",
>> > +				__func__);
>> > +			omap_dvfs_remove_freq_request(dvfs_info,
>> req_dev,
>> > +				target_dev);
>> > +			return ret;
>> > +		}
>> > +	} else {
>>
>> The function comments don't describe this case.  Namely, that if you
>> pass in rate = 0, it removes any previous requests for the requesting
>> device.
> OK. Will add that in function comments.
>>
>> > +		dvfs_info = get_dvfs_info(od->hwmods[0]->voltdm);
>> > +
>> > +		ret = omap_dvfs_remove_freq_request(dvfs_info,
>> req_dev,
>> > +				target_dev);
>> > +		if (ret) {
>> > +			dev_err(target_dev, "%s: Unable to remove
>> frequency request\n",
>> > +				__func__);
>> > +			return ret;
>> > +		}
>> > +
>> > +		ret = omap_dvfs_remove_vdd_user(dvfs_info, req_dev);
>> > +		if (ret) {
>> > +			dev_err(target_dev, "%s: Unable to remove voltage
>> request\n",
>> > +				__func__);
>> > +			return ret;
>> > +		}
>> > +	}
>> > +
>> > +	/* Do the actual scaling */
>> > +	ret = omap_dvfs_voltage_scale(dvfs_info);
>>
>> ok
>>
>> > +	if (!ret)
>> > +		if (omap_device_get_rate(target_dev) >= rate)
>> > +			return 0;
>> > +
>>
>> but this bit needs some explanation.  IIUC: if the _voltage_scale()
>> fails (which also scales the frequency) but the frequency was
>> sucessfully changed, then return success.
> Yes, this is basically to cater for situations where some of the devices
> in the associated vdd have locked their frequencies. In that case, voltage
> scaling would not have happened where as frequency for the requested
> device has been set successfully. In that case return success. Will add
> these in function comments.
>>
>> Also 'rate' is used here where 'freq' would be expected.
> OK. Will fix it.
>
> Vishwa
>>
>> > +	return ret;
>> > +}
>> > +EXPORT_SYMBOL(omap_device_scale);
>> > +
>> > +/**
>> >   * omap_dvfs_init() - Initialize omap dvfs layer
>> >   *
>> >   * Initalizes omap dvfs layer. It basically allocates memory for
>> > diff --git a/arch/arm/plat-omap/include/plat/dvfs.h b/arch/arm/plat-
>> omap/include/plat/dvfs.h
>> > index 1302990..1be2b9d 100644
>> > --- a/arch/arm/plat-omap/include/plat/dvfs.h
>> > +++ b/arch/arm/plat-omap/include/plat/dvfs.h
>> > @@ -17,11 +17,18 @@
>> >
>> >  #ifdef CONFIG_PM
>> >  int omap_dvfs_register_device(struct voltagedomain *voltdm, struct
>> device *dev);
>> > +int omap_device_scale(struct device *req_dev, struct device *dev,
>> > +			unsigned long rate);
>> >  #else
>> >  static inline int omap_dvfs_register_device(struct voltagedomain
>> *voltdm,
>> >  		struct device *dev)
>> >  {
>> >  	return -EINVAL;
>> >  }
>> > +static inline int omap_device_scale(struct device *req_dev, struct
>> devices
>> > +			*target_dev, unsigned long rate);
>> > +{
>> > +	return -EINVAL;
>> > +}
>> >  #endif
>> >  #endif
>>
>> Kevin

RE: [PATCH 04/13] OMAP: Introduce dependent voltage domain support

[Vishwanath Sripathy]

> -----Original Message-----
> From: Kevin Hilman [mailto:khilman@ti.com]
> Sent: Thursday, February 10, 2011 10:07 PM
> To: Vishwanath Sripathy
> Cc: linux-omap@vger.kernel.org; patches@linaro.org; Thara Gopinath
> Subject: Re: [PATCH 04/13] OMAP: Introduce dependent voltage domain
> support
>
> Vishwanath Sripathy <vishwanath.bs@ti.com> writes:
>
> [...]
>
> >> > diff --git a/arch/arm/mach-omap2/dvfs.c b/arch/arm/mach-
> >> omap2/dvfs.c
> >> > index cefc2be..c9d3894 100755
> >> > --- a/arch/arm/mach-omap2/dvfs.c
> >> > +++ b/arch/arm/mach-omap2/dvfs.c
> >> > @@ -85,6 +85,7 @@ struct omap_vdd_dvfs_info {
> >> >  	struct mutex scaling_mutex; /* dvfs mutex */
> >> >  	struct voltagedomain *voltdm;
> >> >  	struct list_head dev_list;
> >> > +	struct device vdd_device;
> >>
> >> It's not clear what the usage of this device is for.
> >>
> >> It is never initialized, but seems to be used as a dummy device when
> >> calcluating dependencies.   Needs clarification.
> >
> > This device is used for placing voltage request
> (omap_dvfs_add_vdd_user)
> > when dealing with dependent vdds. Eg: while scaling MPU VDD, we
> also scale
> > CORE VDD. So while placing voltage request for CORE VDD, this
> vdd_device
> > of MPU is used as the requesting device so that this request is stored
> as
> > a separate user.
>
> I was able to follow how it was used, and why.   But it is still not
> clear to the reader.
>
> First, the device is never initialized, so it's essentially a dummy
> device that remains initialized by default to all zeros.   So, any
> attempt to use any device APIs (e.g. dev_name, etc.) on this will not
> behave as expected.
>
> Second, the reason for this device and the need for the request to be
> stored as a separate user are not clear in the code and don't exist in
> the comments.
OK. I will add more comments to make these details more explicit.

Vishwa
>
> Kevin
>
>
>
>

Re: [PATCH 08/13] OMAP3: cpufreq driver changes for DVFS support

[Kahn, Gery]

Dear Vishwanath,

On Fri, Jan 21, 2011 at 16:01, Vishwanath BS <vishwanath.bs@ti.com> wrote:
> Changes in the omap cpufreq driver for DVFS support.
>
> Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> ---
>  arch/arm/plat-omap/cpu-omap.c |   35 ++++++++++++++++++++++++++++++++---
>  1 files changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/plat-omap/cpu-omap.c b/arch/arm/plat-omap/cpu-omap.c
> index 1c1b80b..d965220 100644
> --- a/arch/arm/plat-omap/cpu-omap.c
> +++ b/arch/arm/plat-omap/cpu-omap.c
> @@ -30,10 +30,12 @@
>  #include <mach/hardware.h>
>  #include <plat/clock.h>
>  #include <asm/system.h>
> +#include <asm/cpu.h>
>
>  #if defined(CONFIG_ARCH_OMAP3) && !defined(CONFIG_OMAP_PM_NONE)
>  #include <plat/omap-pm.h>
>  #include <plat/common.h>
> +#include <plat/dvfs.h>
>  #endif
>
>  #define VERY_HI_RATE   900000000
> @@ -85,11 +87,11 @@ static int omap_target(struct cpufreq_policy *policy,
>                       unsigned int target_freq,
>                       unsigned int relation)
>  {
> -#ifdef CONFIG_ARCH_OMAP1
>        struct cpufreq_freqs freqs;
> -#endif
>  #if defined(CONFIG_ARCH_OMAP3) && !defined(CONFIG_OMAP_PM_NONE)
>        unsigned long freq;
> +       int i;
> +       struct cpufreq_freqs freqs_notify;
>        struct device *mpu_dev = omap2_get_mpuss_device();
>  #endif
>        int ret = 0;
> @@ -116,9 +118,36 @@ static int omap_target(struct cpufreq_policy *policy,
>        ret = clk_set_rate(mpu_clk, freqs.new * 1000);
>        cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
>  #elif defined(CONFIG_ARCH_OMAP3) && !defined(CONFIG_OMAP_PM_NONE)
> +       freqs.old = omap_getspeed(policy->cpu);;
> +       freqs_notify.new = clk_round_rate(mpu_clk, target_freq * 1000) / 1000;
> +       freqs.cpu = policy->cpu;
> +
> +       if (freqs.old == freqs.new)
> +               return ret;
> +
> +       /* pre notifiers */
> +       for_each_cpu(i, policy->cpus) {
> +               freqs.cpu = i;
> +               cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
> +       }
> +
> +       /* scale the frequency */
>        freq = target_freq * 1000;
>        if (opp_find_freq_ceil(mpu_dev, &freq))
> -               omap_pm_cpu_set_freq(freq);
> +               omap_device_scale(mpu_dev, mpu_dev, freq);
> +
> +       /* Update loops per jiffy */
> +       freqs.new = omap_getspeed(policy->cpu);
> +       for_each_cpu(i, policy->cpus)
> +               per_cpu(cpu_data, i).loops_per_jiffy =
> +               cpufreq_scale(per_cpu(cpu_data, i).loops_per_jiffy,
> +                               freqs.old, freqs.new);
> +
> +       /* post notifiers */
> +       for_each_cpu(i, policy->cpus) {
> +               freqs.cpu = i;
> +               cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
> +       }
>  #endif
>        return ret;
>  }
> --
> 1.7.0.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

I took those patches and applied to my .38-rc4 with help from Nishanth Menon.

There is problem appeared during compilation:

arch/arm/plat-omap/cpu-omap.c:142: error: 'struct cpuinfo_arm' has no
member named 'loops_per_jiffy'

My kernel is for Zoom3 board (OMAP3630-GP rev 2, CPU-OPP2 L3-200MHz)
Looking at the source, arch/arm/include/asm/cpu.h
loops_per_jiffy is under CONFIG_SMP and since the omap2plus_defconfig
builds for OMAP4 as well by default, I am guessing this bug was not
immediately visible...

Is there solution for the issue?

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

RE: [PATCH 08/13] OMAP3: cpufreq driver changes for DVFS support

[Vishwanath Sripathy]

> -----Original Message-----
> From: Kahn, Gery [mailto:geryk@ti.com]
> Sent: Monday, February 14, 2011 3:04 PM
> To: Vishwanath BS
> Cc: linux-omap@vger.kernel.org; patches@linaro.org; Santosh Shilimkar
> Subject: Re: [PATCH 08/13] OMAP3: cpufreq driver changes for DVFS
> support
>
> Dear Vishwanath,
>
> On Fri, Jan 21, 2011 at 16:01, Vishwanath BS <vishwanath.bs@ti.com>
> wrote:
> > Changes in the omap cpufreq driver for DVFS support.
> >
> > Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
> > Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> > ---
> >  arch/arm/plat-omap/cpu-omap.c |   35
> ++++++++++++++++++++++++++++++++---
> >  1 files changed, 32 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm/plat-omap/cpu-omap.c b/arch/arm/plat-
> omap/cpu-omap.c
> > index 1c1b80b..d965220 100644
> > --- a/arch/arm/plat-omap/cpu-omap.c
> > +++ b/arch/arm/plat-omap/cpu-omap.c
> > @@ -30,10 +30,12 @@
> >  #include <mach/hardware.h>
> >  #include <plat/clock.h>
> >  #include <asm/system.h>
> > +#include <asm/cpu.h>
> >
> >  #if defined(CONFIG_ARCH_OMAP3) &&
> !defined(CONFIG_OMAP_PM_NONE)
> >  #include <plat/omap-pm.h>
> >  #include <plat/common.h>
> > +#include <plat/dvfs.h>
> >  #endif
> >
> >  #define VERY_HI_RATE   900000000
> > @@ -85,11 +87,11 @@ static int omap_target(struct cpufreq_policy
> *policy,
> >                       unsigned int target_freq,
> >                       unsigned int relation)
> >  {
> > -#ifdef CONFIG_ARCH_OMAP1
> >        struct cpufreq_freqs freqs;
> > -#endif
> >  #if defined(CONFIG_ARCH_OMAP3) &&
> !defined(CONFIG_OMAP_PM_NONE)
> >        unsigned long freq;
> > +       int i;
> > +       struct cpufreq_freqs freqs_notify;
> >        struct device *mpu_dev = omap2_get_mpuss_device();
> >  #endif
> >        int ret = 0;
> > @@ -116,9 +118,36 @@ static int omap_target(struct cpufreq_policy
> *policy,
> >        ret = clk_set_rate(mpu_clk, freqs.new * 1000);
> >        cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
> >  #elif defined(CONFIG_ARCH_OMAP3) &&
> !defined(CONFIG_OMAP_PM_NONE)
> > +       freqs.old = omap_getspeed(policy->cpu);;
> > +       freqs_notify.new = clk_round_rate(mpu_clk, target_freq *
> 1000) / 1000;
> > +       freqs.cpu = policy->cpu;
> > +
> > +       if (freqs.old == freqs.new)
> > +               return ret;
> > +
> > +       /* pre notifiers */
> > +       for_each_cpu(i, policy->cpus) {
> > +               freqs.cpu = i;
> > +               cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
> > +       }
> > +
> > +       /* scale the frequency */
> >        freq = target_freq * 1000;
> >        if (opp_find_freq_ceil(mpu_dev, &freq))
> > -               omap_pm_cpu_set_freq(freq);
> > +               omap_device_scale(mpu_dev, mpu_dev, freq);
> > +
> > +       /* Update loops per jiffy */
> > +       freqs.new = omap_getspeed(policy->cpu);
> > +       for_each_cpu(i, policy->cpus)
> > +               per_cpu(cpu_data, i).loops_per_jiffy =
> > +               cpufreq_scale(per_cpu(cpu_data, i).loops_per_jiffy,
> > +                               freqs.old, freqs.new);
> > +
> > +       /* post notifiers */
> > +       for_each_cpu(i, policy->cpus) {
> > +               freqs.cpu = i;
> > +               cpufreq_notify_transition(&freqs,
> CPUFREQ_POSTCHANGE);
> > +       }
> >  #endif
> >        return ret;
> >  }
> > --
> > 1.7.0.4
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-omap"
in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >
>
> I took those patches and applied to my .38-rc4 with help from Nishanth
> Menon.
>
> There is problem appeared during compilation:
>
> arch/arm/plat-omap/cpu-omap.c:142: error: 'struct cpuinfo_arm' has no
> member named 'loops_per_jiffy'
>
> My kernel is for Zoom3 board (OMAP3630-GP rev 2, CPU-OPP2 L3-
> 200MHz)
> Looking at the source, arch/arm/include/asm/cpu.h
> loops_per_jiffy is under CONFIG_SMP and since the omap2plus_defconfig
> builds for OMAP4 as well by default, I am guessing this bug was not
> immediately visible...
>
> Is there solution for the issue?
Yes this is an issue if CONFIG_SMP is disabled. We probably will need to
put this code under #ifdef CONFIG_SMP till lpj is handled for SMP in
generic cpufreq code itself .

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

Re: [PATCH 08/13] OMAP3: cpufreq driver changes for DVFS support

[Menon, Nishanth]

On Mon, Feb 14, 2011 at 18:19, Vishwanath Sripathy <vishwanath.bs@ti.com> wrote:

>> Is there solution for the issue?
> Yes this is an issue if CONFIG_SMP is disabled. We probably will need to
> put this code under #ifdef CONFIG_SMP till lpj is handled for SMP in
> generic cpufreq code itself .
>
Apologies, I am not clear, but how'd we handle the non smp case here?

Regards,
Nishanth Menon

RE: [PATCH 08/13] OMAP3: cpufreq driver changes for DVFS support

[Vishwanath Sripathy]

> -----Original Message-----
> From: Menon, Nishanth [mailto:nm@ti.com]
> Sent: Monday, February 14, 2011 6:33 PM
> To: Vishwanath Sripathy
> Cc: Gery Kahn; linux-omap@vger.kernel.org; patches@linaro.org;
> Santosh Shilimkar
> Subject: Re: [PATCH 08/13] OMAP3: cpufreq driver changes for DVFS
> support
>
> On Mon, Feb 14, 2011 at 18:19, Vishwanath Sripathy
> <vishwanath.bs@ti.com> wrote:
>
> >> Is there solution for the issue?
> > Yes this is an issue if CONFIG_SMP is disabled. We probably will need
> to
> > put this code under #ifdef CONFIG_SMP till lpj is handled for SMP in
> > generic cpufreq code itself .
> >
> Apologies, I am not clear, but how'd we handle the non smp case here?
For non SMP case, lpj is updated in common cpufreq code (in adjust_jiffies
function).

Vishwa
>
> Regards,
> Nishanth Menon

Re: [PATCH 08/13] OMAP3: cpufreq driver changes for DVFS support

[Kahn, Gery]

Hi Vishwanath,

On Mon, Feb 14, 2011 at 14:49, Vishwanath Sripathy <vishwanath.bs@ti.com> wrote:
>>
>> I took those patches and applied to my .38-rc4 with help from Nishanth
>> Menon.
>>
>> There is problem appeared during compilation:
>>
>> arch/arm/plat-omap/cpu-omap.c:142: error: 'struct cpuinfo_arm' has no
>> member named 'loops_per_jiffy'
>>
>> My kernel is for Zoom3 board (OMAP3630-GP rev 2, CPU-OPP2 L3-
>> 200MHz)
>> Looking at the source, arch/arm/include/asm/cpu.h
>> loops_per_jiffy is under CONFIG_SMP and since the omap2plus_defconfig
>> builds for OMAP4 as well by default, I am guessing this bug was not
>> immediately visible...
>>
>> Is there solution for the issue?
> Yes this is an issue if CONFIG_SMP is disabled. We probably will need to
> put this code under #ifdef CONFIG_SMP till lpj is handled for SMP in
> generic cpufreq code itself .
>
I tried to go this way, but it doesn't help. It may demand additional changes.
> Vishwa
>>
>> Regards,
>> Gery
>
BTW,
i found that this patch provides CPU in 1GHz for omap36xx
is there solution for running omap36xx with 1.2GHz?

Gery

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

[Jarkko Nikula]

On Fri, 28 Jan 2011 16:31:22 -0800
Kevin Hilman <khilman@ti.com> wrote:

> Hi Vishwa,
> 
> Vishwanath BS <vishwanath.bs@ti.com> writes:
> 
> > +/* OMAP 3430 MPU Core VDD dependency table */
> > +static struct omap_vdd_dep_volt omap34xx_vdd1_vdd2_data[] = {
> > +	{.main_vdd_volt = OMAP3430_VDD_MPU_OPP1_UV, .dep_vdd_volt = OMAP4430_VDD_CORE_OPP50_UV},
> > +	{.main_vdd_volt = OMAP3430_VDD_MPU_OPP2_UV, .dep_vdd_volt = OMAP4430_VDD_CORE_OPP50_UV},
> > +	{.main_vdd_volt = OMAP3430_VDD_MPU_OPP3_UV, .dep_vdd_volt = OMAP4430_VDD_CORE_OPP100_UV},
> > +	{.main_vdd_volt = OMAP3430_VDD_MPU_OPP4_UV, .dep_vdd_volt = OMAP4430_VDD_CORE_OPP100_UV},
> > +	{.main_vdd_volt = OMAP3430_VDD_MPU_OPP5_UV, .dep_vdd_volt = OMAP4430_VDD_CORE_OPP100_UV},
> > +	{.main_vdd_volt = 0, .dep_vdd_volt = 0},
> > +};
> 
> This 34xx table has 4430 OPP voltages for CORE, which are clearly not
> right.  34xx has 3 CORE voltages to pick from, so I'm not sure which
> ones are right
> 
> Please update this with the correct 34xx voltages and also validate on
> 34xx also.
> 
Is it known what subsystem dependencies there are?

What I've noticed on N900 that the DSS will stop working if core
voltage is scaled under OMAP3430_VDD_CORE_OPP3_UV. I.e. either if using
OMAP4430_VDD_CORE_OPP50_UV above or OMAP3430_VDD_CORE_OPP2 or 1.

-- 
Jarkko

Re: [PATCH 08/13] OMAP3: cpufreq driver changes for DVFS support

[Jarkko Nikula]

Hi

A few comments below in case this is still under development as I was
playing a bit with this version on omap3.

On Fri, 21 Jan 2011 19:31:00 +0530
Vishwanath BS <vishwanath.bs@ti.com> wrote:

> Changes in the omap cpufreq driver for DVFS support.
> 
> Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> ---
>  arch/arm/plat-omap/cpu-omap.c |   35 ++++++++++++++++++++++++++++++++---
>  1 files changed, 32 insertions(+), 3 deletions(-)
> 
...
> @@ -85,11 +87,11 @@ static int omap_target(struct cpufreq_policy *policy,
>  		       unsigned int target_freq,
>  		       unsigned int relation)
>  {
> -#ifdef CONFIG_ARCH_OMAP1
>  	struct cpufreq_freqs freqs;
> -#endif
>  #if defined(CONFIG_ARCH_OMAP3) && !defined(CONFIG_OMAP_PM_NONE)
>  	unsigned long freq;
> +	int i;
> +	struct cpufreq_freqs freqs_notify;
>  	struct device *mpu_dev = omap2_get_mpuss_device();
>  #endif
>  	int ret = 0;
> @@ -116,9 +118,36 @@ static int omap_target(struct cpufreq_policy *policy,
>  	ret = clk_set_rate(mpu_clk, freqs.new * 1000);
>  	cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
>  #elif defined(CONFIG_ARCH_OMAP3) && !defined(CONFIG_OMAP_PM_NONE)
> +	freqs.old = omap_getspeed(policy->cpu);;
> +	freqs_notify.new = clk_round_rate(mpu_clk, target_freq * 1000) / 1000;
> +	freqs.cpu = policy->cpu;
> +
> +	if (freqs.old == freqs.new)
> +		return ret;
> +

Here freqs.new is uninitialized which very likely means that code falls
always through, sets the correct target frequency though, but can
populate the wrong frequency through the cpufreq_notify_transition when
running the pre notifiers below.

I think the code above meant

freqs_notify.new = clk_round_rate(mpu_clk, target_freq * 1000) / 1000;
->
freqs.new = clk_round_rate(mpu_clk, target_freq * 1000) / 1000;

as the freqs_notify is otherwise unused?

However, that doesn't work as the clk_round_rate returns the current
rate for mpu_clk on omap3 and "freqs.old == freqs.new" is always true.
Looks like there is no round_rate function for arm_fck.

I replaced that with "freqs.new = target_freq;" but this means there
will be unnecessary fall throughs if the real frequency (eg 124800 kHz)
will differ from opp table frequency (eg 125000 kHz) and no real
changes are happening e.g. with on-demand governor.

> +	/* pre notifiers */
> +	for_each_cpu(i, policy->cpus) {
> +		freqs.cpu = i;
> +		cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
> +	}

-- 
Jarkko

RE: [PATCH 08/13] OMAP3: cpufreq driver changes for DVFS support

[Vishwanath Sripathy]

> -----Original Message-----
> From: Jarkko Nikula [mailto:jhnikula@gmail.com]
> Sent: Wednesday, April 13, 2011 7:13 AM
> To: Vishwanath BS
> Cc: linux-omap@vger.kernel.org; patches@linaro.org; Santosh
> Shilimkar
> Subject: Re: [PATCH 08/13] OMAP3: cpufreq driver changes for DVFS
> support
>
> Hi
>
> A few comments below in case this is still under development as I
> was
> playing a bit with this version on omap3.
There is an updated version of omap cpufreq patches where this issue is
fixed.
https://patchwork.kernel.org/patch/632781/

Regards
Vishwa

>
> On Fri, 21 Jan 2011 19:31:00 +0530
> Vishwanath BS <vishwanath.bs@ti.com> wrote:
>
> > Changes in the omap cpufreq driver for DVFS support.
> >
> > Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
> > Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> > ---
> >  arch/arm/plat-omap/cpu-omap.c |   35
> ++++++++++++++++++++++++++++++++---
> >  1 files changed, 32 insertions(+), 3 deletions(-)
> >
> ...
> > @@ -85,11 +87,11 @@ static int omap_target(struct cpufreq_policy
> *policy,
> >  		       unsigned int target_freq,
> >  		       unsigned int relation)
> >  {
> > -#ifdef CONFIG_ARCH_OMAP1
> >  	struct cpufreq_freqs freqs;
> > -#endif
> >  #if defined(CONFIG_ARCH_OMAP3) && !defined(CONFIG_OMAP_PM_NONE)
> >  	unsigned long freq;
> > +	int i;
> > +	struct cpufreq_freqs freqs_notify;
> >  	struct device *mpu_dev = omap2_get_mpuss_device();
> >  #endif
> >  	int ret = 0;
> > @@ -116,9 +118,36 @@ static int omap_target(struct cpufreq_policy
> *policy,
> >  	ret = clk_set_rate(mpu_clk, freqs.new * 1000);
> >  	cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
> >  #elif defined(CONFIG_ARCH_OMAP3) && !defined(CONFIG_OMAP_PM_NONE)
> > +	freqs.old = omap_getspeed(policy->cpu);;
> > +	freqs_notify.new = clk_round_rate(mpu_clk, target_freq * 1000)
> / 1000;
> > +	freqs.cpu = policy->cpu;
> > +
> > +	if (freqs.old == freqs.new)
> > +		return ret;
> > +
>
> Here freqs.new is uninitialized which very likely means that code
> falls
> always through, sets the correct target frequency though, but can
> populate the wrong frequency through the cpufreq_notify_transition
> when
> running the pre notifiers below.
>
> I think the code above meant
>
> freqs_notify.new = clk_round_rate(mpu_clk, target_freq * 1000) /
> 1000;
> ->
> freqs.new = clk_round_rate(mpu_clk, target_freq * 1000) / 1000;
>
> as the freqs_notify is otherwise unused?
>
> However, that doesn't work as the clk_round_rate returns the current
> rate for mpu_clk on omap3 and "freqs.old == freqs.new" is always
> true.
> Looks like there is no round_rate function for arm_fck.
>
> I replaced that with "freqs.new = target_freq;" but this means there
> will be unnecessary fall throughs if the real frequency (eg 124800
> kHz)
> will differ from opp table frequency (eg 125000 kHz) and no real
> changes are happening e.g. with on-demand governor.
>
> > +	/* pre notifiers */
> > +	for_each_cpu(i, policy->cpus) {
> > +		freqs.cpu = i;
> > +		cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
> > +	}
>
> --
> Jarkko

Re: [PATCH 08/13] OMAP3: cpufreq driver changes for DVFS support

[Jarkko Nikula]

On Wed, 13 Apr 2011 10:57:54 -0700
Vishwanath Sripathy <vishwanath.bs@ti.com> wrote:

> > -----Original Message-----
> > From: Jarkko Nikula [mailto:jhnikula@gmail.com]
> > Sent: Wednesday, April 13, 2011 7:13 AM
> > To: Vishwanath BS
> > Cc: linux-omap@vger.kernel.org; patches@linaro.org; Santosh
> > Shilimkar
> > Subject: Re: [PATCH 08/13] OMAP3: cpufreq driver changes for DVFS
> > support
> >
> > Hi
> >
> > A few comments below in case this is still under development as I
> > was
> > playing a bit with this version on omap3.
> There is an updated version of omap cpufreq patches where this issue is
> fixed.
> https://patchwork.kernel.org/patch/632781/
> 
Thanks, I didn't notice that patch fixes the issues I noticed. Now
also the clk_round_rate works fine since the dpll1_ck does have the
round_clock defined and the former arm_fck is anyway derived from it.

-- 
Jarkko

[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