[RFC PATCHv2 0/7] devfreq: Add devfreq-event class to provide raw data for devfreq device

[RFC PATCHv2 0/7] devfreq: Add devfreq-event class to provide raw data for devfreq device

[Chanwoo Choi]

This patchset add new devfreq_event class to provide raw data to determine
current utilization of device  which is used for devfreq governor.

[Description of devfreq-event class]
This patchset add new devfreq_event class for devfreq_event device which provide
raw data (e.g., memory bus utilization/GPU utilization). This raw data from
devfreq_event data would be used for the governor of devfreq subsystem.
- devfreq_event device : Provide raw data for governor of existing devfreq device
- devfreq device       : Monitor device state and change frequency/voltage of device
                         using the raw data from devfreq_event device

The devfreq subsystem support generic DVFS(Dynamic Voltage/Frequency Scaling)
for Non-CPU Devices. The devfreq device would dertermine current device state
using various governor (e.g., ondemand, performance, powersave). After completed
determination of system state, devfreq device would change the frequency/voltage
of devfreq device according to the result of governor.

But, devfreq governor must need basic data which indicates current device state.
Existing devfreq subsystem only consider devfreq device which check current system
state and determine proper system state using basic data. There is no subsystem
for device providing basic data to devfreq device.

The devfreq subsystem must need devfreq_event device(data-provider device) for
existing devfreq device. So, this patch add new devfreq_event class for
devfreq_event device which read various basic data(e.g, memory bus utilization,
GPU utilization) and provide measured data to existing devfreq device through
standard APIs of devfreq_event class.

The following description explains the feature of two kind of devfreq class:
- devfreq class (existing)
 : devfreq consumer device use raw data from devfreq_event device for
   determining proper current system state and change voltage/frequency
   dynamically using various governors.
- devfreq_event class (new)
 : Provide measured raw data to devfreq device for governor

Also, the devfreq-event device would support various type event as following:
 : DEVFREQ_EVENT_TYPE_RAW_DATA
 : DEVFREQ_EVENT_TYPE_UTILIZATION
 : DEVFREQ_EVENT_TYPE_BANDWIDTH
 : DEVFREQ_EVENT_TYPE_LATENCY

[For example]
If board dts includes PPMU_DMC0/DMC1/CPU event node,
would show following sysfs entry. Also devfreq driver(e.g., exynos4_bus.c)
can get the instance of devfreq-event device by using provided API and then
get raw data which reflect the current state of device.

-sh-3.2# pwd
/sys/class/devfreq_event
-sh-3.2# ls -al
total 0
drwxr-xr-x  2 root root 0 Jan  7 11:10 .
drwxr-xr-x 37 root root 0 Jan  7 11:10 ..
lrwxrwxrwx  1 root root 0 Jan  7 11:10 event.0 -> ../../devices/soc/106a0000.ppmu_dmc0/devfreq_event/event.0
lrwxrwxrwx  1 root root 0 Jan  7 11:10 event.1 -> ../../devices/soc/106a0000.ppmu_dmc0/devfreq_event/event.1
lrwxrwxrwx  1 root root 0 Jan  7 11:10 event.2 -> ../../devices/soc/106a0000.ppmu_dmc0/devfreq_event/event.2
lrwxrwxrwx  1 root root 0 Jan  7 11:10 event.3 -> ../../devices/soc/106a0000.ppmu_dmc0/devfreq_event/event.3
lrwxrwxrwx  1 root root 0 Jan  7 11:10 event.4 -> ../../devices/soc/106b0000.ppmu_dmc1/devfreq_event/event.4
lrwxrwxrwx  1 root root 0 Jan  7 11:10 event.5 -> ../../devices/soc/106c0000.ppmu_cpu/devfreq_event/event.5
lrwxrwxrwx  1 root root 0 Jan  7 11:10 event.6 -> ../../devices/soc/106c0000.ppmu_cpu/devfreq_event/event.6
lrwxrwxrwx  1 root root 0 Jan  7 11:10 event.7 -> ../../devices/soc/106c0000.ppmu_cpu/devfreq_event/event.7
lrwxrwxrwx  1 root root 0 Jan  7 11:10 event.8 -> ../../devices/soc/106c0000.ppmu_cpu/devfreq_event/event.8

Changes from v1:
- Code clean
- Add the description of devfreq-event structure
- Add 'is_enabled' function to devfreq_event_ops structure
- Add 'enable_count' field to devfreq_event_dev structure
- Check whether devfreq-event device is enabled or not
  during calling devfreq_event API
- Define the type of devfreq-event device as following
  : DEVFREQ_EVENT_TYPE_RAW_DATA
  : DEVFREQ_EVENT_TYPE_UTILIZATION
  : DEVFREQ_EVENT_TYPE_BANDWIDTH
  : DEVFREQ_EVENT_TYPE_LATENCY
- Add the exclusive feature of devfreq-event device.
  If devfreq-event device is used on only on devfreq driver,
  should used 'devfreq_enable_event_dev_exclusive()' function
- Add new patch6 for test on Exynos3250-based Rinato board

Chanwoo Choi (7):
  devfreq: event: Add new devfreq_event class to provide basic data for devfreq governor
  devfreq: event: Add the list of supported devfreq-event type
  devfreq: event: Add the exclusive flag for devfreq-event device
  devfreq: event: Add exynos-ppmu devfreq event driver
  ARM: dts: Add PPMU dt node for Exynos3250
  ARM: dts: Add PPMU dt node for Exynos4 SoC
  ARM: dts: exynos: Add PPMU dt node to Exynos3250-based Rinato board

 arch/arm/boot/dts/exynos3250-rinato.dts |  55 +++++
 arch/arm/boot/dts/exynos3250.dtsi       |  66 ++++++
 arch/arm/boot/dts/exynos4.dtsi          |  96 ++++++++
 arch/arm/boot/dts/exynos4210.dtsi       |   8 +
 drivers/devfreq/Kconfig                 |  11 +
 drivers/devfreq/Makefile                |   5 +-
 drivers/devfreq/devfreq-event.c         | 363 ++++++++++++++++++++++++++++
 drivers/devfreq/event/Makefile          |   2 +
 drivers/devfreq/event/exynos-ppmu.c     | 409 ++++++++++++++++++++++++++++++++
 include/linux/devfreq.h                 | 178 ++++++++++++++
 10 files changed, 1192 insertions(+), 1 deletion(-)
 create mode 100644 drivers/devfreq/devfreq-event.c
 create mode 100644 drivers/devfreq/event/Makefile
 create mode 100644 drivers/devfreq/event/exynos-ppmu.c

-- 
1.8.5.5

[RFC PATCHv2 1/7] devfreq: event: Add new devfreq_event class to provide basic data for devfreq governor

[Chanwoo Choi]

This patch add new devfreq_event class for devfreq_event device which provide
raw data (e.g., memory bus utilization/GPU utilization). This raw data from
devfreq_event data would be used for the governor of devfreq subsystem.
- devfreq_event device : Provide raw data for governor of existing devfreq device
- devfreq device       : Monitor device state and change frequency/voltage of device
                         using the raw data from devfreq_event device

The devfreq subsystem support generic DVFS(Dynamic Voltage/Frequency Scaling)
for Non-CPU Devices. The devfreq device would dertermine current device state
using various governor (e.g., ondemand, performance, powersave). After completed
determination of system state, devfreq device would change the frequency/voltage
of devfreq device according to the result of governor.

But, devfreq governor must need basic data which indicates current device state.
Existing devfreq subsystem only consider devfreq device which check current system
state and determine proper system state using basic data. There is no subsystem
for device providing basic data to devfreq device.

The devfreq subsystem must need devfreq_event device(data-provider device) for
existing devfreq device. So, this patch add new devfreq_event class for
devfreq_event device which read various basic data(e.g, memory bus utilization,
GPU utilization) and provide measured data to existing devfreq device through
standard APIs of devfreq_event class.

The following description explains the feature of two kind of devfreq class:
- devfreq class (existing)
 : devfreq consumer device use raw data from devfreq_event device for
   determining proper current system state and change voltage/frequency
   dynamically using various governors.

- devfreq_event class (new)
 : Provide measured raw data to devfreq device for governor

Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
---
 drivers/devfreq/Kconfig         |   2 +
 drivers/devfreq/Makefile        |   5 +-
 drivers/devfreq/devfreq-event.c | 302 ++++++++++++++++++++++++++++++++++++++++
 drivers/devfreq/event/Makefile  |   1 +
 include/linux/devfreq.h         | 141 +++++++++++++++++++
 5 files changed, 450 insertions(+), 1 deletion(-)
 create mode 100644 drivers/devfreq/devfreq-event.c
 create mode 100644 drivers/devfreq/event/Makefile

diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
index faf4e70..4d15b62 100644
--- a/drivers/devfreq/Kconfig
+++ b/drivers/devfreq/Kconfig
@@ -87,4 +87,6 @@ config ARM_EXYNOS5_BUS_DEVFREQ
 	  It reads PPMU counters of memory controllers and adjusts the
 	  operating frequencies and voltages with OPP support.
 
+comment "DEVFREQ Event Drivers"
+
 endif # PM_DEVFREQ
diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
index 16138c9..a1ffabe 100644
--- a/drivers/devfreq/Makefile
+++ b/drivers/devfreq/Makefile
@@ -1,4 +1,4 @@
-obj-$(CONFIG_PM_DEVFREQ)	+= devfreq.o
+obj-$(CONFIG_PM_DEVFREQ)		+= devfreq.o devfreq-event.o
 obj-$(CONFIG_DEVFREQ_GOV_SIMPLE_ONDEMAND)	+= governor_simpleondemand.o
 obj-$(CONFIG_DEVFREQ_GOV_PERFORMANCE)	+= governor_performance.o
 obj-$(CONFIG_DEVFREQ_GOV_POWERSAVE)	+= governor_powersave.o
@@ -7,3 +7,6 @@ obj-$(CONFIG_DEVFREQ_GOV_USERSPACE)	+= governor_userspace.o
 # DEVFREQ Drivers
 obj-$(CONFIG_ARM_EXYNOS4_BUS_DEVFREQ)	+= exynos/
 obj-$(CONFIG_ARM_EXYNOS5_BUS_DEVFREQ)	+= exynos/
+
+# DEVFREQ Event Drivers
+obj-$(CONFIG_PM_DEVFREQ)		+= event/
diff --git a/drivers/devfreq/devfreq-event.c b/drivers/devfreq/devfreq-event.c
new file mode 100644
index 0000000..b47329f
--- /dev/null
+++ b/drivers/devfreq/devfreq-event.c
@@ -0,0 +1,302 @@
+/*
+ * devfreq-event: Generic DEVFREQ Event class driver
+ *
+ * Copyright (C) 2014 Samsung Electronics
+ * Chanwoo Choi <cw00.choi@samsung.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.
+ *
+ * This driver is based on drivers/devfreq/devfreq.c
+ */
+
+#include <linux/kernel.h>
+#include <linux/sched.h>
+#include <linux/errno.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/stat.h>
+#include <linux/pm_opp.h>
+#include <linux/devfreq.h>
+#include <linux/workqueue.h>
+#include <linux/platform_device.h>
+#include <linux/list.h>
+#include <linux/printk.h>
+#include <linux/hrtimer.h>
+#include <linux/of.h>
+#include "governor.h"
+
+static struct class *devfreq_event_class;
+
+/* The list of all devfreq event list */
+static LIST_HEAD(devfreq_event_list);
+static DEFINE_MUTEX(devfreq_event_list_lock);
+
+#define to_devfreq_event(DEV) container_of(DEV, struct devfreq_event_dev, dev)
+
+struct devfreq_event_dev *devfreq_add_event_device(struct device *dev,
+					struct devfreq_event_desc *desc)
+{
+	struct devfreq_event_dev *event_dev;
+	static atomic_t event_no = ATOMIC_INIT(0);
+	int ret;
+
+	if (!dev || !desc)
+		return ERR_PTR(-EINVAL);
+
+	if (!desc->name || !desc->ops)
+		return ERR_PTR(-EINVAL);
+
+	event_dev = kzalloc(sizeof(struct devfreq_event_dev), GFP_KERNEL);
+	if (!event_dev)
+		return ERR_PTR(-ENOMEM);
+
+	mutex_lock(&devfreq_event_list_lock);
+
+	mutex_init(&event_dev->lock);
+	event_dev->desc = desc;
+	event_dev->dev.parent = dev;
+	event_dev->dev.class = devfreq_event_class;
+
+	dev_set_name(&event_dev->dev, "event.%d",
+		     atomic_inc_return(&event_no) - 1);
+	ret = device_register(&event_dev->dev);
+	if (ret != 0) {
+		put_device(&event_dev->dev);
+		goto err;
+	}
+	dev_set_drvdata(&event_dev->dev, event_dev);
+
+	/* Add devfreq event device to devfreq_event_list */
+	INIT_LIST_HEAD(&event_dev->node);
+	list_add(&event_dev->node, &devfreq_event_list);
+
+	mutex_unlock(&devfreq_event_list_lock);
+
+	return event_dev;
+err:
+	kfree(event_dev);
+	return ERR_PTR(ret);
+}
+EXPORT_SYMBOL(devfreq_add_event_device);
+
+struct devfreq_event_dev *devfreq_get_event_dev(const char *event_dev_name)
+{
+	struct devfreq_event_dev *event_dev;
+
+	mutex_lock(&devfreq_event_list_lock);
+	list_for_each_entry(event_dev, &devfreq_event_list, node) {
+		if (!strcmp(event_dev->desc->name, event_dev_name))
+			goto out;
+	}
+	event_dev = NULL;
+out:
+	mutex_unlock(&devfreq_event_list_lock);
+
+	return event_dev;
+}
+EXPORT_SYMBOL_GPL(devfreq_get_event_dev);
+
+struct devfreq_event_dev *devfreq_get_event_dev_by_phandle(struct device *dev,
+						      int index)
+{
+	struct device_node *node;
+	struct devfreq_event_dev *event_dev;
+
+	if (!dev->of_node) {
+		dev_err(dev, "device does not have a device node entry\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	node = of_parse_phandle(dev->of_node, "devfreq-events", index);
+	if (!node) {
+		dev_err(dev, "failed to get phandle in %s node\n",
+			dev->of_node->full_name);
+		return ERR_PTR(-ENODEV);
+	}
+
+	event_dev = devfreq_get_event_dev(node->name);
+	if (!event_dev) {
+		dev_err(dev, "unable to get devfreq-event device : %s\n",
+			node->name);
+		return ERR_PTR(-ENODEV);
+	}
+
+	return event_dev;
+}
+EXPORT_SYMBOL_GPL(devfreq_get_event_dev_by_phandle);
+
+int devfreq_put_event_dev(struct devfreq_event_dev *event_dev)
+{
+	return 0;
+}
+EXPORT_SYMBOL_GPL(devfreq_put_event_dev);
+
+int devfreq_enable_event_dev(struct devfreq_event_dev *event_dev)
+{
+	int ret = 0;
+
+	if (!event_dev || !event_dev->desc)
+		return -EINVAL;
+
+	mutex_lock(&event_dev->lock);
+	if (event_dev->desc->ops && event_dev->desc->ops->enable) {
+		ret = event_dev->desc->ops->enable(event_dev);
+		if (ret < 0)
+			goto err;
+	}
+	event_dev->enable_count++;
+err:
+	mutex_unlock(&event_dev->lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(devfreq_enable_event_dev);
+
+int devfreq_disable_event_dev(struct devfreq_event_dev *event_dev)
+{
+	int ret = 0;
+
+	if (!event_dev || !event_dev->desc)
+		return -EINVAL;
+
+	mutex_lock(&event_dev->lock);
+	if (event_dev->enable_count > 0) {
+		event_dev->enable_count--;
+	} else {
+		dev_warn(&event_dev->dev, "unbalanced enable_count\n");
+		ret = -EINVAL;
+		goto err;
+	}
+
+	if (event_dev->desc->ops && event_dev->desc->ops->disable) {
+		ret = event_dev->desc->ops->disable(event_dev);
+		if (ret < 0) {
+			event_dev->enable_count++;
+			goto err;
+		}
+	}
+err:
+	mutex_unlock(&event_dev->lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(devfreq_disable_event_dev);
+
+bool devfreq_is_enabled_event_dev(struct devfreq_event_dev *event_dev)
+{
+	bool enabled = false;
+
+	if (!event_dev || !event_dev->desc)
+		return enabled;
+
+	if (!(event_dev->enable_count > 0))
+		return enabled;
+
+	if (event_dev->desc->ops && event_dev->desc->ops->is_enabled)
+		enabled = event_dev->desc->ops->is_enabled(event_dev);
+
+	return enabled;
+}
+EXPORT_SYMBOL_GPL(devfreq_is_enabled_event_dev);
+
+int devfreq_set_event_event_dev(struct devfreq_event_dev *event_dev)
+{
+	if (!event_dev || !event_dev->desc)
+		return -EINVAL;
+
+	if (!devfreq_is_enabled_event_dev(event_dev))
+		return -EPERM;
+
+	if (event_dev->desc->ops && event_dev->desc->ops->set_event)
+		return event_dev->desc->ops->set_event(event_dev);
+
+	return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(devfreq_set_event_event_dev);
+
+int devfreq_get_event_event_dev(struct devfreq_event_dev *event_dev,
+				int *total_event)
+{
+	if (!event_dev || !event_dev->desc)
+		return -EINVAL;
+
+	if (!devfreq_is_enabled_event_dev(event_dev))
+		return -EPERM;
+
+	if (event_dev->desc->ops && event_dev->desc->ops->get_event)
+		return event_dev->desc->ops->get_event(event_dev, total_event);
+
+	return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(devfreq_get_event_event_dev);
+
+int devfreq_reset_event_dev(struct devfreq_event_dev *event_dev)
+{
+	if (!event_dev || !event_dev->desc)
+		return -EINVAL;
+
+	if (!devfreq_is_enabled_event_dev(event_dev))
+		return -EPERM;
+
+	if (event_dev->desc->ops && event_dev->desc->ops->reset)
+		return event_dev->desc->ops->reset(event_dev);
+
+	return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(devfreq_reset_event_dev);
+
+void *event_dev_get_drvdata(struct devfreq_event_dev *event_dev)
+{
+	return event_dev->desc->driver_data;
+}
+EXPORT_SYMBOL_GPL(event_dev_get_drvdata);
+
+/*
+ * Device Attribues for devfreq event class
+ */
+static ssize_t name_show(struct device *dev,
+				       struct device_attribute *attr,
+				       char *buf)
+{
+	struct devfreq_event_dev *event_dev = to_devfreq_event(dev);
+
+	if (!event_dev || !event_dev->desc)
+		return -EINVAL;
+
+	return sprintf(buf, "%s\n", event_dev->desc->name);
+}
+static DEVICE_ATTR_RO(name);
+
+static struct attribute *devfreq_event_attrs[] = {
+	&dev_attr_name.attr,
+	NULL,
+};
+ATTRIBUTE_GROUPS(devfreq_event);
+
+static int __init devfreq_event_init(void)
+{
+	devfreq_event_class = class_create(THIS_MODULE, "devfreq_event");
+	if (IS_ERR(devfreq_event_class)) {
+		pr_err("%s: couldn't create class\n", __FILE__);
+		return PTR_ERR(devfreq_event_class);
+	}
+
+	devfreq_event_class->dev_groups = devfreq_event_groups;
+
+	return 0;
+}
+subsys_initcall(devfreq_event_init);
+
+static void __exit devfreq_event_exit(void)
+{
+	class_destroy(devfreq_event_class);
+}
+module_exit(devfreq_event_exit);
+
+MODULE_AUTHOR("Chanwoo Choi <cw00.choi@samsung.com>");
+MODULE_DESCRIPTION("devfreq-event class support");
+MODULE_LICENSE("GPL");
diff --git a/drivers/devfreq/event/Makefile b/drivers/devfreq/event/Makefile
new file mode 100644
index 0000000..dc56005
--- /dev/null
+++ b/drivers/devfreq/event/Makefile
@@ -0,0 +1 @@
+# Exynos DEVFREQ Event Drivers
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index f1863dc..3c5f233 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -175,6 +175,68 @@ struct devfreq {
 	unsigned long last_stat_updated;
 };
 
+/**
+ * struct devfreq_event_dev - the devfreq-event device 
+ *
+ * @node	: Contain the devfreq-event device that have been registered.
+ * @dev		: the device registered by devfreq-event class. dev.parent is
+ *		  the device using devfreq-event.
+ * @lock	: a mutex to protect accessing devfreq-event.
+ * @enable_count: the number of enable function have been called.
+ * @desc	: the description for devfreq-event device.
+ *
+ * This structure contains devfreq-event device information.
+ */
+struct devfreq_event_dev {
+	struct list_head node;
+
+	struct device dev;
+	struct mutex lock;
+	u32 enable_count;
+
+	const struct devfreq_event_desc *desc;
+};
+
+/**
+ * struct devfreq_event_ops - the operations of devfreq-event device
+ *
+ * @enable	: Enable the devfreq-event device.
+ * @disable	: Disable the devfreq-event device.
+ * @is_enabled	: Return true if the devfreq-event is enabled, false if not.
+ * @set_event	: Set the specific event type for the devfreq-event device.
+ * @get_event	: Get the result of the devfreq-event devie with specific
+ *		  event type.
+ * @reset	: Reset all setting of the devfreq-event device.
+ *
+ * This structure contains devfreq-event device operations which can be
+ * implemented by devfreq-event device drivers.
+ */
+struct devfreq_event_ops {
+	int (*enable)(struct devfreq_event_dev *event_dev);
+	int (*disable)(struct devfreq_event_dev *event_dev);
+	bool (*is_enabled)(struct devfreq_event_dev *event_dev);
+	int (*set_event)(struct devfreq_event_dev *event_dev);
+	int (*get_event)(struct devfreq_event_dev *event_dev, int *total_event);
+	int (*reset)(struct devfreq_event_dev *event_dev);
+};
+
+/**
+ * struct devfreq_event_desc - the descriptor of devfreq-event device
+ *
+ * @name	: the name of devfreq-event device.
+ * @driver_data	: the private data for devfreq-event driver.
+ * @ops		: the operation to control devfreq-event device.
+ *
+ * Each devfreq-event device is described with a this structure.
+ * This structure contains the various data for devfreq-event device.
+ */
+struct devfreq_event_desc {
+	const char *name;
+	void *driver_data;
+
+	struct devfreq_event_ops *ops;
+};
+
 #if defined(CONFIG_PM_DEVFREQ)
 extern struct devfreq *devfreq_add_device(struct device *dev,
 				  struct devfreq_dev_profile *profile,
@@ -204,6 +266,22 @@ extern int devm_devfreq_register_opp_notifier(struct device *dev,
 extern void devm_devfreq_unregister_opp_notifier(struct device *dev,
 						struct devfreq *devfreq);
 
+/* Functions for devfreq event device */
+extern struct devfreq_event_dev *devfreq_add_event_device(struct device *dev,
+				struct devfreq_event_desc *desc);
+extern void *event_dev_get_drvdata(struct devfreq_event_dev *event_dev);
+extern struct devfreq_event_dev *devfreq_get_event_dev(const char *name);
+extern struct devfreq_event_dev *devfreq_get_event_dev_by_phandle(
+				struct device *dev, int index);
+extern int devfreq_put_event_dev(struct devfreq_event_dev *event_dev);
+extern int devfreq_enable_event_dev(struct devfreq_event_dev *event_dev);
+extern int devfreq_disable_event_dev(struct devfreq_event_dev *event_dev);
+extern bool devfreq_is_enabled_event_dev(struct devfreq_event_dev *event_dev);
+extern int devfreq_set_event_event_dev(struct devfreq_event_dev *event_dev);
+extern int devfreq_get_event_event_dev(struct devfreq_event_dev *event_dev,
+					int *total_event);
+extern int devfreq_reset_event_dev(struct devfreq_event_dev *event_dev);
+
 #if IS_ENABLED(CONFIG_DEVFREQ_GOV_SIMPLE_ONDEMAND)
 /**
  * struct devfreq_simple_ondemand_data - void *data fed to struct devfreq
@@ -233,6 +311,13 @@ static inline struct devfreq *devfreq_add_device(struct device *dev,
 	return ERR_PTR(-ENOSYS);
 }
 
+static inline struct devfreq_event_dev *devfreq_add_event_device(
+					struct device *dev,
+					struct devfreq_event_desc *desc);
+{
+	return ERR_PTR(-ENODEV);
+}
+
 static inline int devfreq_remove_device(struct devfreq *devfreq)
 {
 	return 0;
@@ -289,6 +374,62 @@ static inline void devm_devfreq_unregister_opp_notifier(struct device *dev,
 							struct devfreq *devfreq)
 {
 }
+
+static inline void *event_dev_get_drvdata(struct devfreq_event_dev *event_dev)
+{
+	return ERR_PTR(-EINVAL);
+}
+
+static inline struct devfreq_event_dev *devfreq_get_event_dev(const char *name)
+{
+	return ERR_PTR(-ENODEV);
+}
+
+struct devfreq_event_dev *devfreq_get_event_dev_by_phandle(struct device *dev,
+						      int index)
+{
+	return ERR_PTR(-ENODEV);
+}
+
+static inline int devfreq_put_event_dev(struct devfreq_event_dev *event_dev)
+{
+	return -EINVAL;
+}
+
+static inline int devfreq_enable_event_dev(struct devfreq_event_dev *event_dev)
+{
+	return -EINVAL;
+}
+
+static inline int devfreq_disable_event_dev(
+				struct devfreq_event_dev *event_dev)
+{
+	return -EINVAL;
+}
+
+static inline bool devfreq_is_enabled_event_dev(
+				struct devfreq_event_dev *event_dev)
+{
+	return false;
+}
+
+static inline int devfreq_set_event_event_dev(
+				struct devfreq_event_dev *event_dev)
+{
+	return -EINVAL;
+}
+
+static inline int devfreq_get_event_event_dev(
+				struct devfreq_event_dev *event_dev,
+				int *total_event);
+{
+	return -EINVAL;
+}
+
+static inline int devfreq_reset_event_dev(struct devfreq_event_dev *event_dev)
+{
+	return -EINVAL;
+}
 #endif /* CONFIG_PM_DEVFREQ */
 
 #endif /* __LINUX_DEVFREQ_H__ */
-- 
1.8.5.5

[RFC PATCHv2 2/7] devfreq: event: Add the list of supported devfreq-event type

[Chanwoo Choi]

This patch adds the list of supported devfreq-event type as following.
Each devfreq-event device driver would support the various devfreq-event type
for devfreq governor at the same time.
- DEVFREQ_EVENT_TYPE_RAW_DATA
- DEVFREQ_EVENT_TYPE_UTILIZATION
- DEVFREQ_EVENT_TYPE_BANDWIDTH
- DEVFREQ_EVENT_TYPE_LATENCY

Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
---
 drivers/devfreq/devfreq-event.c | 23 ++++++++++++++++++++---
 include/linux/devfreq.h         | 28 ++++++++++++++++++++++++----
 2 files changed, 44 insertions(+), 7 deletions(-)

diff --git a/drivers/devfreq/devfreq-event.c b/drivers/devfreq/devfreq-event.c
index b47329f..a71fcee 100644
--- a/drivers/devfreq/devfreq-event.c
+++ b/drivers/devfreq/devfreq-event.c
@@ -203,7 +203,8 @@ bool devfreq_is_enabled_event_dev(struct devfreq_event_dev *event_dev)
 }
 EXPORT_SYMBOL_GPL(devfreq_is_enabled_event_dev);
 
-int devfreq_set_event_event_dev(struct devfreq_event_dev *event_dev)
+int devfreq_set_event_event_dev(struct devfreq_event_dev *event_dev,
+				enum devfreq_event_type event_type)
 {
 	if (!event_dev || !event_dev->desc)
 		return -EINVAL;
@@ -211,14 +212,22 @@ int devfreq_set_event_event_dev(struct devfreq_event_dev *event_dev)
 	if (!devfreq_is_enabled_event_dev(event_dev))
 		return -EPERM;
 
+	if ((event_dev->desc->event_type & event_type) == 0) {
+		dev_err(&event_dev->dev, "unsupported of devfreq-event type\n");
+		return -EINVAL;
+	}
+
 	if (event_dev->desc->ops && event_dev->desc->ops->set_event)
-		return event_dev->desc->ops->set_event(event_dev);
+		return event_dev->desc->ops->set_event(event_dev, event_type);
+
+	dev_err(&event_dev->dev, "failed to set devfreq-event\n");
 
 	return -EINVAL;
 }
 EXPORT_SYMBOL_GPL(devfreq_set_event_event_dev);
 
 int devfreq_get_event_event_dev(struct devfreq_event_dev *event_dev,
+				enum devfreq_event_type event_type,
 				int *total_event)
 {
 	if (!event_dev || !event_dev->desc)
@@ -227,8 +236,16 @@ int devfreq_get_event_event_dev(struct devfreq_event_dev *event_dev,
 	if (!devfreq_is_enabled_event_dev(event_dev))
 		return -EPERM;
 
+	if ((event_dev->desc->event_type & event_type) == 0) {
+		dev_err(&event_dev->dev, "unsupported of devfreq-event type\n");
+		return -EINVAL;
+	}
+
 	if (event_dev->desc->ops && event_dev->desc->ops->get_event)
-		return event_dev->desc->ops->get_event(event_dev, total_event);
+		return event_dev->desc->ops->get_event(event_dev, event_type,
+							total_event);
+
+	dev_err(&event_dev->dev, "failed to get devfreq-event\n");
 
 	return -EINVAL;
 }
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index 3c5f233..f6d44b7 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -197,6 +197,15 @@ struct devfreq_event_dev {
 	const struct devfreq_event_desc *desc;
 };
 
+/* The supported type by devfreq-event device */
+enum devfreq_event_type {
+	DEVFREQ_EVENT_TYPE_NONE		= BIT(0),
+	DEVFREQ_EVENT_TYPE_RAW_DATA	= BIT(1),
+	DEVFREQ_EVENT_TYPE_UTILIZATION	= BIT(2),
+	DEVFREQ_EVENT_TYPE_BANDWIDTH	= BIT(3),
+	DEVFREQ_EVENT_TYPE_LATENCY	= BIT(4),
+};
+
 /**
  * struct devfreq_event_ops - the operations of devfreq-event device
  *
@@ -215,8 +224,10 @@ struct devfreq_event_ops {
 	int (*enable)(struct devfreq_event_dev *event_dev);
 	int (*disable)(struct devfreq_event_dev *event_dev);
 	bool (*is_enabled)(struct devfreq_event_dev *event_dev);
-	int (*set_event)(struct devfreq_event_dev *event_dev);
-	int (*get_event)(struct devfreq_event_dev *event_dev, int *total_event);
+	int (*set_event)(struct devfreq_event_dev *event_dev,
+			enum devfreq_event_type);
+	int (*get_event)(struct devfreq_event_dev *event_dev,
+			enum devfreq_event_type, int *total_event);
 	int (*reset)(struct devfreq_event_dev *event_dev);
 };
 
@@ -225,6 +236,10 @@ struct devfreq_event_ops {
  *
  * @name	: the name of devfreq-event device.
  * @driver_data	: the private data for devfreq-event driver.
+ * @event_type	: the supported devfreq-event type among as following
+ *		  - DEVFREQ_EVENT_TYPE_UTILIZATION
+ *		  - DEVFREQ_EVENT_TYPE_BANDWIDTH
+ *		  - DEVFREQ_EVENT_TYPE_LATENCY
  * @ops		: the operation to control devfreq-event device.
  *
  * Each devfreq-event device is described with a this structure.
@@ -233,6 +248,7 @@ struct devfreq_event_ops {
 struct devfreq_event_desc {
 	const char *name;
 	void *driver_data;
+	enum devfreq_event_type event_type;
 
 	struct devfreq_event_ops *ops;
 };
@@ -277,8 +293,10 @@ extern int devfreq_put_event_dev(struct devfreq_event_dev *event_dev);
 extern int devfreq_enable_event_dev(struct devfreq_event_dev *event_dev);
 extern int devfreq_disable_event_dev(struct devfreq_event_dev *event_dev);
 extern bool devfreq_is_enabled_event_dev(struct devfreq_event_dev *event_dev);
-extern int devfreq_set_event_event_dev(struct devfreq_event_dev *event_dev);
+extern int devfreq_set_event_event_dev(struct devfreq_event_dev *event_dev,
+					enum devfreq_event_type event_type);
 extern int devfreq_get_event_event_dev(struct devfreq_event_dev *event_dev,
+					enum devfreq_event_type event_type,
 					int *total_event);
 extern int devfreq_reset_event_dev(struct devfreq_event_dev *event_dev);
 
@@ -414,13 +432,15 @@ static inline bool devfreq_is_enabled_event_dev(
 }
 
 static inline int devfreq_set_event_event_dev(
-				struct devfreq_event_dev *event_dev)
+				struct devfreq_event_dev *event_dev,
+				enum devfreq_event_type event_type);
 {
 	return -EINVAL;
 }
 
 static inline int devfreq_get_event_event_dev(
 				struct devfreq_event_dev *event_dev,
+				enum devfreq_event_type event_type,
 				int *total_event);
 {
 	return -EINVAL;
-- 
1.8.5.5

[RFC PATCHv2 3/7] devfreq: event: Add the exclusive flag for devfreq-event device

[Chanwoo Choi]

This patch adds the exclusive flag for devfreq-event device. If the specific
devfreq-event device should be used in only one devfreq device driver,
devfreq driver have to use 'devfreq_enable_event_dev_exclusive()' function
to consist integrity of event data.

Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
---
 drivers/devfreq/devfreq-event.c | 44 +++++++++++++++++++++++++++++++++++++++++
 include/linux/devfreq.h         | 17 ++++++++++++++++
 2 files changed, 61 insertions(+)

diff --git a/drivers/devfreq/devfreq-event.c b/drivers/devfreq/devfreq-event.c
index a71fcee..e7ce5ba 100644
--- a/drivers/devfreq/devfreq-event.c
+++ b/drivers/devfreq/devfreq-event.c
@@ -142,6 +142,9 @@ int devfreq_enable_event_dev(struct devfreq_event_dev *event_dev)
 	if (!event_dev || !event_dev->desc)
 		return -EINVAL;
 
+	if (event_dev->event_flag & DEVFREQ_EVENT_FLAG_EXCLUSIVE)
+		return -EBUSY;
+
 	mutex_lock(&event_dev->lock);
 	if (event_dev->desc->ops && event_dev->desc->ops->enable) {
 		ret = event_dev->desc->ops->enable(event_dev);
@@ -156,6 +159,29 @@ err:
 }
 EXPORT_SYMBOL_GPL(devfreq_enable_event_dev);
 
+int devfreq_enable_event_dev_exclusive(struct devfreq_event_dev *event_dev)
+{
+	int ret;
+
+	if (!event_dev || !event_dev->desc)
+		return -EINVAL;
+
+	if (event_dev->event_flag & DEVFREQ_EVENT_FLAG_EXCLUSIVE)
+		return -EBUSY;
+
+	ret = devfreq_enable_event_dev(event_dev);
+	if (ret < 0)
+		return ret;
+
+	/* Set exclusive flag to devfreq-event device */
+	mutex_lock(&event_dev->lock);
+	event_dev->event_flag |= DEVFREQ_EVENT_FLAG_EXCLUSIVE;
+	mutex_unlock(&event_dev->lock);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(devfreq_enable_event_dev_exclusive);
+
 int devfreq_disable_event_dev(struct devfreq_event_dev *event_dev)
 {
 	int ret = 0;
@@ -163,6 +189,9 @@ int devfreq_disable_event_dev(struct devfreq_event_dev *event_dev)
 	if (!event_dev || !event_dev->desc)
 		return -EINVAL;
 
+	if (event_dev->event_flag & DEVFREQ_EVENT_FLAG_EXCLUSIVE)
+		return -EBUSY;
+
 	mutex_lock(&event_dev->lock);
 	if (event_dev->enable_count > 0) {
 		event_dev->enable_count--;
@@ -196,6 +225,9 @@ bool devfreq_is_enabled_event_dev(struct devfreq_event_dev *event_dev)
 	if (!(event_dev->enable_count > 0))
 		return enabled;
 
+	if (event_dev->event_flag & DEVFREQ_EVENT_FLAG_EXCLUSIVE)
+		return -EBUSY;
+
 	if (event_dev->desc->ops && event_dev->desc->ops->is_enabled)
 		enabled = event_dev->desc->ops->is_enabled(event_dev);
 
@@ -212,6 +244,9 @@ int devfreq_set_event_event_dev(struct devfreq_event_dev *event_dev,
 	if (!devfreq_is_enabled_event_dev(event_dev))
 		return -EPERM;
 
+	if (event_dev->event_flag & DEVFREQ_EVENT_FLAG_EXCLUSIVE)
+		return -EBUSY;
+
 	if ((event_dev->desc->event_type & event_type) == 0) {
 		dev_err(&event_dev->dev, "unsupported of devfreq-event type\n");
 		return -EINVAL;
@@ -236,6 +271,9 @@ int devfreq_get_event_event_dev(struct devfreq_event_dev *event_dev,
 	if (!devfreq_is_enabled_event_dev(event_dev))
 		return -EPERM;
 
+	if (event_dev->event_flag & DEVFREQ_EVENT_FLAG_EXCLUSIVE)
+		return -EBUSY;
+
 	if ((event_dev->desc->event_type & event_type) == 0) {
 		dev_err(&event_dev->dev, "unsupported of devfreq-event type\n");
 		return -EINVAL;
@@ -259,6 +297,9 @@ int devfreq_reset_event_dev(struct devfreq_event_dev *event_dev)
 	if (!devfreq_is_enabled_event_dev(event_dev))
 		return -EPERM;
 
+	if (event_dev->event_flag & DEVFREQ_EVENT_FLAG_EXCLUSIVE)
+		return -EBUSY;
+
 	if (event_dev->desc->ops && event_dev->desc->ops->reset)
 		return event_dev->desc->ops->reset(event_dev);
 
@@ -268,6 +309,9 @@ EXPORT_SYMBOL_GPL(devfreq_reset_event_dev);
 
 void *event_dev_get_drvdata(struct devfreq_event_dev *event_dev)
 {
+	if (event_dev->event_flag & DEVFREQ_EVENT_FLAG_EXCLUSIVE)
+		return ERR_PTR(-EBUSY);
+
 	return event_dev->desc->driver_data;
 }
 EXPORT_SYMBOL_GPL(event_dev_get_drvdata);
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index f6d44b7..e7fedea 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -175,6 +175,12 @@ struct devfreq {
 	unsigned long last_stat_updated;
 };
 
+/* The supported flags by devfreq-event device */
+enum devfreq_event_flag {
+	DEVFREQ_EVENT_FLAG_NONE		= BIT(0),
+	DEVFREQ_EVENT_FLAG_EXCLUSIVE	= BIT(1),
+};
+
 /**
  * struct devfreq_event_dev - the devfreq-event device 
  *
@@ -194,6 +200,9 @@ struct devfreq_event_dev {
 	struct mutex lock;
 	u32 enable_count;
 
+	enum devfreq_event_flag event_flag;
+	unsigned int enabled_count;
+
 	const struct devfreq_event_desc *desc;
 };
 
@@ -291,6 +300,8 @@ extern struct devfreq_event_dev *devfreq_get_event_dev_by_phandle(
 				struct device *dev, int index);
 extern int devfreq_put_event_dev(struct devfreq_event_dev *event_dev);
 extern int devfreq_enable_event_dev(struct devfreq_event_dev *event_dev);
+extern int devfreq_enable_event_dev_exclusive(
+					struct devfreq_event_dev *event_dev);
 extern int devfreq_disable_event_dev(struct devfreq_event_dev *event_dev);
 extern bool devfreq_is_enabled_event_dev(struct devfreq_event_dev *event_dev);
 extern int devfreq_set_event_event_dev(struct devfreq_event_dev *event_dev,
@@ -419,6 +430,12 @@ static inline int devfreq_enable_event_dev(struct devfreq_event_dev *event_dev)
 	return -EINVAL;
 }
 
+static inline int devfreq_enable_event_dev_exclusive(
+					struct devfreq_event_dev *event_dev)
+{
+	return -EINVAL;
+}
+
 static inline int devfreq_disable_event_dev(
 				struct devfreq_event_dev *event_dev)
 {
-- 
1.8.5.5

[RFC PATCHv2 4/7] devfreq: event: Add exynos-ppmu devfreq event driver

[Chanwoo Choi]

This patch add exynos-ppmu devfreq event driver to provider raw data about
the utilization of each IP in Exynos SoC series.

Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
---
 drivers/devfreq/Kconfig             |   9 +
 drivers/devfreq/event/Makefile      |   1 +
 drivers/devfreq/event/exynos-ppmu.c | 409 ++++++++++++++++++++++++++++++++++++
 3 files changed, 419 insertions(+)
 create mode 100644 drivers/devfreq/event/exynos-ppmu.c

diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
index 4d15b62..d4559f7 100644
--- a/drivers/devfreq/Kconfig
+++ b/drivers/devfreq/Kconfig
@@ -89,4 +89,13 @@ config ARM_EXYNOS5_BUS_DEVFREQ
 
 comment "DEVFREQ Event Drivers"
 
+config DEVFREQ_EVENT_EXYNOS_PPMU
+	bool "EXYNOS PPMU (Performance Profiling Monitoring Unit) DEVFREQ event Driver"
+	depends on ARCH_EXYNOS
+	select PM_OPP
+	help
+	 This add the DEVFREQ event driver for Exynos SoC. It provides PPMU
+	 (Performance Profiling Monitoring Unit) counters to estimate the
+	 utilization of each module.
+
 endif # PM_DEVFREQ
diff --git a/drivers/devfreq/event/Makefile b/drivers/devfreq/event/Makefile
index dc56005..be146ea 100644
--- a/drivers/devfreq/event/Makefile
+++ b/drivers/devfreq/event/Makefile
@@ -1 +1,2 @@
 # Exynos DEVFREQ Event Drivers
+obj-$(CONFIG_DEVFREQ_EVENT_EXYNOS_PPMU) += exynos-ppmu.o
diff --git a/drivers/devfreq/event/exynos-ppmu.c b/drivers/devfreq/event/exynos-ppmu.c
new file mode 100644
index 0000000..2706f23
--- /dev/null
+++ b/drivers/devfreq/event/exynos-ppmu.c
@@ -0,0 +1,409 @@
+/*
+ * exynos_ppmu.c - EXYNOS PPMU (Performance Profiling Monitoring Units) support
+ *
+ * Copyright (c) 2014 Samsung Electronics Co., Ltd.
+ * Author : Chanwoo Choi <cw00.choi@samsung.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.
+ *
+ * This driver is based on drivers/devfreq/exynos/exynos_ppmu.c
+ */
+
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/suspend.h>
+#include <linux/devfreq.h>
+
+#define PPMU_ENABLE             BIT(0)
+#define PPMU_DISABLE            0x0
+#define PPMU_CYCLE_RESET        BIT(1)
+#define PPMU_COUNTER_RESET      BIT(2)
+
+#define PPMU_ENABLE_COUNT0      BIT(0)
+#define PPMU_ENABLE_COUNT1      BIT(1)
+#define PPMU_ENABLE_COUNT2      BIT(2)
+#define PPMU_ENABLE_COUNT3      BIT(3)
+#define PPMU_ENABLE_CYCLE       BIT(31)
+
+#define PPMU_CNTENS		0x10
+#define PPMU_FLAG		0x50
+#define PPMU_CCNT_OVERFLOW	BIT(31)
+#define PPMU_CCNT		0x100
+
+#define PPMU_PMCNT0		0x110
+#define PPMU_PMCNT_OFFSET	0x10
+#define PMCNT_OFFSET(x)		(PPMU_PMCNT0 + (PPMU_PMCNT_OFFSET * x))
+
+#define PPMU_BEVT0SEL		0x1000
+#define PPMU_BEVTSEL_OFFSET	0x100
+#define PPMU_BEVTSEL(x)		(PPMU_BEVT0SEL + (x * PPMU_BEVTSEL_OFFSET))
+
+#define RD_DATA_COUNT		0x5
+#define WR_DATA_COUNT		0x6
+#define RDWR_DATA_COUNT		0x7
+
+enum ppmu_counter {
+	PPMU_PMNCNT0,
+	PPMU_PMNCNT1,
+	PPMU_PMNCNT2,
+	PPMU_PMNCNT3,
+	PPMU_PMNCNT_MAX,
+};
+
+/* Platform data */
+struct exynos_ppmu_data {
+	struct devfreq *devfreq;
+	struct devfreq_event_dev **event_dev;
+	struct devfreq_event_desc *desc;
+	unsigned int num_events;
+
+	struct device *dev;
+	struct clk *clk_ppmu;
+	struct mutex lock;
+
+	struct __exynos_ppmu {
+		void __iomem *hw_base;
+		unsigned int ccnt;
+		unsigned int event[PPMU_PMNCNT_MAX];
+		unsigned int count[PPMU_PMNCNT_MAX];
+		unsigned long long ns;
+		ktime_t reset_time;
+		bool ccnt_overflow;
+		bool count_overflow[PPMU_PMNCNT_MAX];
+	} ppmu;
+};
+
+struct __exynos_ppmu_events {
+	char *name;
+	int id;
+} ppmu_events[] = {
+	{ "ppmu-dmc0-pmcnt0", PPMU_PMNCNT0 },
+	{ "ppmu-dmc0-pmcnt1", PPMU_PMNCNT1 },
+	{ "ppmu-dmc0-pmcnt2", PPMU_PMNCNT2 },
+	{ "ppmu-dmc0-pmcnt3", PPMU_PMNCNT3 },
+
+	{ "ppmu-dmc1-pmcnt0", PPMU_PMNCNT0 },
+	{ "ppmu-dmc1-pmcnt1", PPMU_PMNCNT1 },
+	{ "ppmu-dmc1-pmcnt2", PPMU_PMNCNT2 },
+	{ "ppmu-dmc1-pmcnt3", PPMU_PMNCNT3 },
+
+	{ "ppmu-cpu-pmcnt0", PPMU_PMNCNT0 },
+	{ "ppmu-cpu-pmcnt1", PPMU_PMNCNT1 },
+	{ "ppmu-cpu-pmcnt2", PPMU_PMNCNT2 },
+	{ "ppmu-cpu-pmcnt3", PPMU_PMNCNT3 },
+
+	{ "ppmu-rightbus-pmcnt0", PPMU_PMNCNT0 },
+	{ "ppmu-rightbus-pmcnt1", PPMU_PMNCNT1 },
+	{ "ppmu-rightbus-pmcnt2", PPMU_PMNCNT2 },
+	{ "ppmu-rightbus-pmcnt3", PPMU_PMNCNT3 },
+
+	{ "ppmu-leftbus-pmcnt0", PPMU_PMNCNT0 },
+	{ "ppmu-leftbus-pmcnt1", PPMU_PMNCNT1 },
+	{ "ppmu-leftbus-pmcnt2", PPMU_PMNCNT2 },
+	{ "ppmu-leftbus-pmcnt3", PPMU_PMNCNT3 },
+
+	{ "ppmu-camif-pmcnt0", PPMU_PMNCNT0 },
+	{ "ppmu-camif-pmcnt1", PPMU_PMNCNT1 },
+	{ "ppmu-camif-pmcnt2", PPMU_PMNCNT2 },
+	{ "ppmu-camif-pmcnt3", PPMU_PMNCNT3 },
+
+	{ "ppmu-lcd0-pmcnt0", PPMU_PMNCNT0 },
+	{ "ppmu-lcd0-pmcnt1", PPMU_PMNCNT1 },
+	{ "ppmu-lcd0-pmcnt2", PPMU_PMNCNT2 },
+	{ "ppmu-lcd0-pmcnt3", PPMU_PMNCNT3 },
+
+	{ "ppmu-g3d-pmcnt0", PPMU_PMNCNT0 },
+	{ "ppmu-g3d-pmcnt1", PPMU_PMNCNT1 },
+	{ "ppmu-g3d-pmcnt2", PPMU_PMNCNT2 },
+	{ "ppmu-g3d-pmcnt3", PPMU_PMNCNT3 },
+
+	{ "ppmu-mfc-pmcnt0", PPMU_PMNCNT0 },
+	{ "ppmu-mfc-pmcnt1", PPMU_PMNCNT1 },
+	{ "ppmu-mfc-pmcnt2", PPMU_PMNCNT2 },
+	{ "ppmu-mfc-pmcnt3", PPMU_PMNCNT3 },
+
+	{ "ppmu-fsys-pmcnt0", PPMU_PMNCNT0 },
+	{ "ppmu-fsys-pmcnt1", PPMU_PMNCNT1 },
+	{ "ppmu-fsys-pmcnt2", PPMU_PMNCNT2 },
+	{ "ppmu-fsys-pmcnt3", PPMU_PMNCNT3 },
+	{ /* sentinel */ },
+};
+
+static int exynos_ppmu_enable(struct devfreq_event_dev *event_dev)
+{
+	struct exynos_ppmu_data *exynos_ppmu = event_dev_get_drvdata(event_dev);
+
+	__raw_writel(PPMU_ENABLE, exynos_ppmu->ppmu.hw_base);
+
+	return 0;
+}
+
+static int exynos_ppmu_disable(struct devfreq_event_dev *event_dev)
+{
+	struct exynos_ppmu_data *exynos_ppmu = event_dev_get_drvdata(event_dev);
+
+	__raw_writel(PPMU_DISABLE, exynos_ppmu->ppmu.hw_base);
+
+	return 0;
+}
+
+static bool exynos_ppmu_is_enabled(struct devfreq_event_dev *event_dev)
+{
+	struct exynos_ppmu_data *exynos_ppmu = event_dev_get_drvdata(event_dev);
+	int val;
+
+	val = __raw_readl(exynos_ppmu->ppmu.hw_base);
+	if (val & PPMU_ENABLE)
+		return true;
+
+	return false;
+}
+
+static int exynos_ppmu_find_ppmu_id(struct devfreq_event_dev *event_dev)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(ppmu_events); i++)
+		if (!strcmp(event_dev->desc->name, ppmu_events[i].name))
+			return ppmu_events[i].id;
+
+	return -EINVAL;
+}
+
+static int exynos_ppmu_set_event(struct devfreq_event_dev *event_dev,
+				enum devfreq_event_type event_type)
+{
+	struct exynos_ppmu_data *exynos_ppmu = event_dev_get_drvdata(event_dev);
+	void __iomem *ppmu_base = exynos_ppmu->ppmu.hw_base;
+	int id = exynos_ppmu_find_ppmu_id(event_dev);
+
+	__raw_writel(RDWR_DATA_COUNT, ppmu_base + PPMU_BEVTSEL(id));
+
+	return 0;
+}
+
+static int exynos_ppmu_get_event(struct devfreq_event_dev *event_dev,
+				enum devfreq_event_type event_type,
+				int *total_event)
+{
+	struct exynos_ppmu_data *exynos_ppmu = event_dev_get_drvdata(event_dev);
+	void __iomem *ppmu_base = exynos_ppmu->ppmu.hw_base;
+	int id = exynos_ppmu_find_ppmu_id(event_dev);
+	int cnt, total_cnt;
+
+	total_cnt = __raw_readl(ppmu_base + PPMU_CCNT);
+
+	if (id == PPMU_PMNCNT3)
+		cnt = ((__raw_readl(ppmu_base + PMCNT_OFFSET(id)) << 8) |
+			  __raw_readl(ppmu_base + PMCNT_OFFSET(id + 1)));
+	else
+		cnt = __raw_readl(ppmu_base + PMCNT_OFFSET(id));
+
+	*total_event = total_cnt;
+
+	return cnt;
+}
+
+static int exynos_ppmu_reset(struct devfreq_event_dev *event_dev)
+{
+	struct exynos_ppmu_data *exynos_ppmu = event_dev_get_drvdata(event_dev);
+	void __iomem *ppmu_base = exynos_ppmu->ppmu.hw_base;
+
+	__raw_writel(PPMU_CYCLE_RESET | PPMU_COUNTER_RESET, ppmu_base);
+	__raw_writel(PPMU_ENABLE_CYCLE  |
+		     PPMU_ENABLE_COUNT0 |
+		     PPMU_ENABLE_COUNT1 |
+		     PPMU_ENABLE_COUNT2 |
+		     PPMU_ENABLE_COUNT3,
+		     ppmu_base + PPMU_CNTENS);
+	__raw_writel(PPMU_ENABLE, exynos_ppmu->ppmu.hw_base);
+
+	return 0;
+}
+
+static struct devfreq_event_ops exynos_ppmu_ops = {
+	.enable		= exynos_ppmu_enable,
+	.disable	= exynos_ppmu_disable,
+	.is_enabled	= exynos_ppmu_is_enabled,
+	.set_event	= exynos_ppmu_set_event,
+	.get_event	= exynos_ppmu_get_event,
+	.reset		= exynos_ppmu_reset,
+};
+
+static int of_get_devfreq_events(struct device_node *np,
+				struct exynos_ppmu_data *exynos_ppmu)
+{
+	struct devfreq_event_desc *desc;
+	struct device *dev = exynos_ppmu->dev;
+	struct device_node *events_np, *node;
+	int i, j, count;
+
+	events_np = of_find_node_by_name(np, "events");
+	if (!events_np) {
+		dev_err(dev, "Failed to find ppmus sub-node\n");
+		return -EINVAL;
+	}
+
+	count = of_get_child_count(events_np);
+	desc = devm_kzalloc(dev, sizeof(struct devfreq_event_desc) * count,
+			      GFP_KERNEL);
+	if (!desc)
+		return -ENOMEM;
+	exynos_ppmu->num_events = count;
+
+	j = 0;
+	for_each_child_of_node(events_np, node) {
+		for (i = 0; i < ARRAY_SIZE(ppmu_events); i++) {
+			if (!of_node_cmp(node->name, ppmu_events[i].name))
+				break;
+		}
+
+		if (i == ARRAY_SIZE(ppmu_events)) {
+			dev_warn(dev,
+				"don't know how to configure events : %s\n",
+				node->name);
+			continue;
+		}
+		desc[j].ops = &exynos_ppmu_ops;
+		desc[j].driver_data = exynos_ppmu;
+		desc[j].event_type |= DEVFREQ_EVENT_TYPE_RAW_DATA;
+		of_property_read_string(node, "event-name", &desc[j].name);
+		j++;
+	}
+	exynos_ppmu->desc = desc;
+
+	return 0;
+}
+
+static int exynos_ppmu_parse_dt(struct exynos_ppmu_data *exynos_ppmu)
+{
+	struct device *dev = exynos_ppmu->dev;
+	struct device_node *np = dev->of_node;
+	int ret = 0;
+
+	if (!np) {
+		dev_err(dev, "Failed to find devicetree node\n");
+		return -EINVAL;
+	}
+
+	/* Maps the memory mapped IO to control PPMU register */
+	exynos_ppmu->ppmu.hw_base = of_iomap(np, 0);
+	if (IS_ERR_OR_NULL(exynos_ppmu->ppmu.hw_base)) {
+		dev_err(dev, "Failed to map memory region\n");
+		ret = -EINVAL;
+		goto err_iomap;
+	}
+
+	/* FIXME: Get the clock of ppmu and enable this clock */
+	exynos_ppmu->clk_ppmu = devm_clk_get(dev, "ppmu");
+	if (IS_ERR(exynos_ppmu->clk_ppmu))
+		dev_warn(dev, "Failed to get PPMU clock\n");
+
+	ret = of_get_devfreq_events(np, exynos_ppmu);
+	if (ret < 0) {
+		dev_err(dev, "Failed to parse exynos ppmu dt node\n");
+		goto err_clock;
+	}
+
+	return 0;
+
+err_clock:
+	clk_disable_unprepare(exynos_ppmu->clk_ppmu);
+err_iomap:
+	iounmap(exynos_ppmu->ppmu.hw_base);
+
+	return ret;
+}
+
+static int exynos_ppmu_probe(struct platform_device *pdev)
+{
+	struct exynos_ppmu_data *exynos_ppmu;
+	struct devfreq_event_dev **event_dev;
+	struct devfreq_event_desc *desc;
+	int i, ret = 0, size;
+
+	/* Allocate the memory of exynos_ppmu_data and initialize it */
+	exynos_ppmu = devm_kzalloc(&pdev->dev, sizeof(struct exynos_ppmu_data),
+				   GFP_KERNEL);
+	if (!exynos_ppmu)
+		return -ENOMEM;
+
+	mutex_init(&exynos_ppmu->lock);
+	exynos_ppmu->dev = &pdev->dev;
+
+	/* Parse dt data to get resource */
+	ret = exynos_ppmu_parse_dt(exynos_ppmu);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Failed to parse DT node for resource\n");
+		return ret;
+	}
+	desc = exynos_ppmu->desc;
+
+	size = sizeof(struct devfreq_event_dev *) * exynos_ppmu->num_events;
+	exynos_ppmu->event_dev = devm_kzalloc(&pdev->dev, size, GFP_KERNEL);
+	if (!exynos_ppmu->event_dev) {
+		dev_err(&pdev->dev,
+			"Failed to allocate memory devfreq_event_dev list\n");
+		return -ENOMEM;
+	}
+	event_dev = exynos_ppmu->event_dev;
+	platform_set_drvdata(pdev, exynos_ppmu);
+
+	for (i = 0; i < exynos_ppmu->num_events; i++) {
+		event_dev[i] = devfreq_add_event_device(&pdev->dev, &desc[i]);
+		if (IS_ERR(event_dev)) {
+			ret = PTR_ERR(event_dev);
+			dev_err(&pdev->dev, "Failed to add devfreq evt dev\n");
+			goto err;
+		}
+	}
+
+	return 0;
+err:
+	clk_disable_unprepare(exynos_ppmu->clk_ppmu);
+	iounmap(exynos_ppmu->ppmu.hw_base);
+
+	return ret;
+}
+
+static int exynos_ppmu_remove(struct platform_device *pdev)
+{
+	struct exynos_ppmu_data *exynos_ppmu = platform_get_drvdata(pdev);
+
+	clk_disable_unprepare(exynos_ppmu->clk_ppmu);
+	iounmap(exynos_ppmu->ppmu.hw_base);
+
+	/* Remove devfreq instance */
+	devfreq_remove_device(exynos_ppmu->devfreq);
+
+	return 0;
+}
+
+static struct of_device_id exynos_ppmu_id_match[] = {
+	{ .compatible = "samsung,exynos-ppmu", },
+	{ /* sentinel */ },
+};
+
+static struct platform_driver exynos_ppmu_driver = {
+	.probe	= exynos_ppmu_probe,
+	.remove	= exynos_ppmu_remove,
+	.driver = {
+		.name	= "exynos-ppmu",
+		.owner	= THIS_MODULE,
+		.of_match_table = exynos_ppmu_id_match,
+	},
+};
+
+module_platform_driver(exynos_ppmu_driver);
+
+MODULE_DESCRIPTION("EXYNOS PPMU(Performance Profiling Monitoring Unit) driver");
+MODULE_AUTHOR("Chanwoo Choi <cw00.choi@samsung.com>");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:exynos-ppmu");
-- 
1.8.5.5

[RFC PATCHv2 5/7] ARM: dts: Add PPMU dt node for Exynos3250

[Chanwoo Choi]

This patch add PPMU (Performance Profiling Monitoring Units) dt node
to estimate the utilization of each IP in Exynos SoC throught DEVFREQ Event
subsystem.

This patch adds following PPMU dt nodes:
- PPMU_DMC0	0x106a0000
- PPMU_DMC1	0x106b0000
- PPMU_RIGHTBUS 0x112A0000
- PPMU_LEFTBUS  0x116A0000
- PPMU_CAMIF    0x11AC0000
- PPMU_LCD0     0x11E40000
- PPMU_3D       0x13220000
- PPMU_MFC_L    0x13660000
- PPMU_CPU	0x106c0000

Cc: Kukjin Kim <kgene.kim@samsung.com>
Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/boot/dts/exynos3250.dtsi | 66 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/arch/arm/boot/dts/exynos3250.dtsi b/arch/arm/boot/dts/exynos3250.dtsi
index c743f60..70a9120 100644
--- a/arch/arm/boot/dts/exynos3250.dtsi
+++ b/arch/arm/boot/dts/exynos3250.dtsi
@@ -529,6 +529,72 @@
 			compatible = "arm,cortex-a7-pmu";
 			interrupts = <0 18 0>, <0 19 0>;
 		};
+
+		ppmu_dmc0: ppmu_dmc0@106a0000 {
+			compatible = "samsung,exynos-ppmu";
+			reg = <0x106a0000 0x2000>;
+			status = "disabled";
+		};
+
+		ppmu_dmc1: ppmu_dmc1@106b0000 {
+			compatible = "samsung,exynos-ppmu";
+			reg = <0x106b0000 0x2000>;
+			status = "disabled";
+		};
+
+		ppmu_cpu: ppmu_cpu@106c0000 {
+			compatible = "samsung,exynos-ppmu";
+			reg = <0x106c0000 0x2000>;
+			status = "disabled";
+		};
+
+		ppmu_rightbus: ppmu_rightbus@112a0000 {
+			compatible = "samsung,exynos-ppmu";
+			reg = <0x112a0000 0x2000>;
+			clocks = <&cmu CLK_PPMURIGHT>;
+			clock-names = "ppmu";
+			status = "disabled";
+		};
+
+		ppmu_leftbus: ppmu_leftbus0@116a0000 {
+			compatible = "samsung,exynos-ppmu";
+			reg = <0x116a0000 0x2000>;
+			clocks = <&cmu CLK_PPMULEFT>;
+			clock-names = "ppmu";
+			status = "disabled";
+		};
+
+		ppmu_camif: ppmu_camif@11ac0000 {
+			compatible = "samsung,exynos-ppmu";
+			reg = <0x11ac0000 0x2000>;
+			clocks = <&cmu CLK_PPMUCAMIF>;
+			clock-names = "ppmu";
+			status = "disabled";
+		};
+
+		ppmu_lcd0: ppmu_lcd0@11e40000 {
+			compatible = "samsung,exynos-ppmu";
+			reg = <0x11e40000 0x2000>;
+			clocks = <&cmu CLK_PPMULCD0>;
+			clock-names = "ppmu";
+			status = "disabled";
+		};
+
+		ppmu_g3d: ppmu_g3d@13220000 {
+			compatible = "samsung,exynos-ppmu";
+			reg = <0x13220000 0x2000>;
+			clocks = <&cmu CLK_PPMUG3D>;
+			clock-names = "ppmu";
+			status = "disabled";
+		};
+
+		ppmu_mfc_l: ppmu_mfc_l@13660000 {
+			compatible = "samsung,exynos-ppmu";
+			reg = <0x13660000 0x2000>;
+			clocks = <&cmu CLK_PPMUMFC_L>;
+			clock-names = "ppmu";
+			status = "disabled";
+		};
 	};
 };
 
-- 
1.8.5.5

[RFC PATCHv2 6/7] ARM: dts: Add PPMU dt node for Exynos4 SoC

[Chanwoo Choi]

This patch add PPMU (Performance Profiling Monitoring Unit) dt node for Exynos4
(Exynos4210/4212/4412) SoC. PPMU dt node is used to monitor the utilization of
each IP.

The Exynos4210/Exynos4212/Exynos4412 SoC includes following PPMUs:
- PPMU_DMC0      0x106A_0000
- PPMU_DMC1      0x106B_0000
- PPMU_CPU       0x106C_0000
- PPMU_ACP       0x10AE_0000
- PPMU_RIGHT_BUS 0x112A_0000
- PPMU_LEFT_BUS  0x116A_0000
- PPMU_LCD0      0x11E4_0000
- PPMU_CAMIF     0x11AC_0000
- PPMU_IMAGE     0x12AA_0000
- PPMU_TV        0x12E4_0000
- PPMU_3D        0x1322_0000
- PPMU_MFC_L     0x1366_0000
- PPMU_MFC_R     0x1367_0000

Additionally, the Exynos4210 SoC includes following PPMUs:
- PPMU_LCD1      0x1224_0000

Cc: Kukjin Kim <kgene.kim@samsung.com>
Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/boot/dts/exynos4.dtsi    | 96 +++++++++++++++++++++++++++++++++++++++
 arch/arm/boot/dts/exynos4210.dtsi |  8 ++++
 2 files changed, 104 insertions(+)

diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi
index e0278ec..150bb56 100644
--- a/arch/arm/boot/dts/exynos4.dtsi
+++ b/arch/arm/boot/dts/exynos4.dtsi
@@ -645,4 +645,100 @@
 		samsung,sysreg = <&sys_reg>;
 		status = "disabled";
 	};
+
+	ppmu_dmc0: ppmu_dmc0@106a0000 {
+		compatible = "samsung,exynos-ppmu";
+		reg = <0x106a0000 0x2000>;
+		clocks = <&clock CLK_PPMUDMC0>;
+		clock-names = "ppmu";
+		status = "disabled";
+	};
+
+	ppmu_dmc1: ppmu_dmc1@106b0000 {
+		compatible = "samsung,exynos-ppmu";
+		reg = <0x106b0000 0x2000>;
+		clocks = <&clock CLK_PPMUDMC1>;
+		clock-names = "ppmu";
+		status = "disabled";
+	};
+
+	ppmu_cpu: ppmu_cpu@106c0000 {
+		compatible = "samsung,exynos-ppmu";
+		reg = <0x106c0000 0x2000>;
+		clocks = <&clock CLK_PPMUCPU>;
+		clock-names = "ppmu";
+		status = "disabled";
+	};
+
+	ppmu_rightbus: ppmu_rightbus@112a0000 {
+		compatible = "samsung,exynos-ppmu";
+		reg = <0x112a0000 0x2000>;
+		clocks = <&clock CLK_PPMURIGHT>;
+		clock-names = "ppmu";
+		status = "disabled";
+	};
+
+	ppmu_leftbus: ppmu_leftbus0@116a0000 {
+		compatible = "samsung,exynos-ppmu";
+		reg = <0x116a0000 0x2000>;
+		clocks = <&clock CLK_PPMULEFT>;
+		clock-names = "ppmu";
+		status = "disabled";
+	};
+
+	ppmu_camif: ppmu_camif@11ac0000 {
+		compatible = "samsung,exynos-ppmu";
+		reg = <0x11ac0000 0x2000>;
+		clocks = <&clock CLK_PPMUCAMIF>;
+		clock-names = "ppmu";
+		status = "disabled";
+	};
+
+	ppmu_lcd0: ppmu_lcd0@11e40000 {
+		compatible = "samsung,exynos-ppmu";
+		reg = <0x11e40000 0x2000>;
+		clocks = <&clock CLK_PPMULCD0>;
+		clock-names = "ppmu";
+		status = "disabled";
+	};
+
+	ppmu_image: ppmu_image@12aa0000 {
+		compatible = "samsung,exynos-ppmu";
+		reg = <0x12aa0000 0x2000>;
+		clocks = <&clock CLK_PPMUIMAGE>;
+		clock-names = "ppmu";
+		status = "disabled";
+	};
+
+	ppmu_tv: ppmu_tv@12e40000 {
+		compatible = "samsung,exynos-ppmu";
+		reg = <0x12e40000 0x2000>;
+		clocks = <&clock CLK_PPMUTV>;
+		clock-names = "ppmu";
+		status = "disabled";
+	};
+
+	ppmu_g3d: ppmu_g3d@13220000 {
+		compatible = "samsung,exynos-ppmu";
+		reg = <0x13220000 0x2000>;
+		clocks = <&clock CLK_PPMUG3D>;
+		clock-names = "ppmu";
+		status = "disabled";
+	};
+
+	ppmu_mfc_l: ppmu_mfc_l@13660000 {
+		compatible = "samsung,exynos-ppmu";
+		reg = <0x13660000 0x2000>;
+		clocks = <&clock CLK_PPMUMFC_L>;
+		clock-names = "ppmu";
+		status = "disabled";
+	};
+
+	ppmu_mfc_r: ppmu_mfc_r@13670000 {
+		compatible = "samsung,exynos-ppmu";
+		reg = <0x13670000 0x2000>;
+		clocks = <&clock CLK_PPMUMFC_R>;
+		clock-names = "ppmu";
+		status = "disabled";
+	};
 };
diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi
index bcc9e63..b2598de 100644
--- a/arch/arm/boot/dts/exynos4210.dtsi
+++ b/arch/arm/boot/dts/exynos4210.dtsi
@@ -192,4 +192,12 @@
 			samsung,lcd-wb;
 		};
 	};
+
+	ppmu_lcd1: ppmu_lcd1@12240000 {
+		compatible = "samsung,exynos-ppmu";
+		reg = <0x12240000 0x2000>;
+		clocks = <&clock CLK_PPMULCD1>;
+		clock-names = "ppmu";
+		status = "disabled";
+	};
 };
-- 
1.8.5.5

[RFC PATCHv2 7/7] ARM: dts: exynos: Add PPMU dt node to Exynos3250-based Rinato board

[Chanwoo Choi]

This patch add PPMU dt node to Exynos3250-base Rinato board. The PPMU dt node
is used to get the utilization of DMC0/DMC1/CPU Block.

Cc: Kukjin Kim <kgene.kim@samsung.com>
Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/boot/dts/exynos3250-rinato.dts | 55 +++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/arch/arm/boot/dts/exynos3250-rinato.dts b/arch/arm/boot/dts/exynos3250-rinato.dts
index 7789442..9a0ffe4 100644
--- a/arch/arm/boot/dts/exynos3250-rinato.dts
+++ b/arch/arm/boot/dts/exynos3250-rinato.dts
@@ -542,6 +542,61 @@
 	status = "okay";
 };
 
+&ppmu_cpu {
+	status = "okay";
+
+	events {
+		ppmu_cpu_0: ppmu-cpu-pmcnt0 {
+			event-name = "ppmu-cpu-pmcnt0";
+		};
+
+		ppmu_cpu_1: ppmu-cpu-pmcnt1 {
+			event-name = "ppmu-cpu-pmcnt1";
+		};
+
+		ppmu_cpu_2: ppmu-cpu-pmcnt2 {
+			event-name = "ppmu-cpu-pmcnt2";
+		};
+
+		ppmu_cpu_3: ppmu-cpu-pmcnt3 {
+			event-name = "ppmu-cpu-pmcnt3";
+		};
+	};
+};
+
+&ppmu_dmc0 {
+	status = "okay";
+
+	events {
+		ppmu_dmc0_0: ppmu-dmc0-pmcnt0 {
+			event-name = "ppmu-dmc0-pmcnt0";
+		};
+
+
+		ppmu_dmc0_1: ppmu-dmc0-pmcnt1 {
+			event-name = "ppmu-dmc0-pmcnt1";
+		};
+
+		ppmu_dmc0_2: ppmu-dmc0-pmcnt2 {
+			event-name = "ppmu-dmc0-pmcnt2";
+		};
+
+		ppmu_dmc0_3: ppmu-dmc0-pmcnt3 {
+			event-name = "ppmu-dmc0-pmcnt3";
+		};
+	};
+};
+
+&ppmu_dmc1 {
+	status = "okay";
+
+	events {
+		ppmu_dmc1_3: ppmu-dmc1-pmcnt3 {
+			event-name = "ppmu-dmc1-pmcnt3";
+		};
+	};
+};
+
 &xusbxti {
 	clock-frequency = <24000000>;
 };
-- 
1.8.5.5

Re: [RFC PATCHv2 1/7] devfreq: event: Add new devfreq_event class to provide basic data for devfreq governor

[Krzysztof Kozlowski]

On wto, 2014-12-09 at 23:13 +0900, Chanwoo Choi wrote:
> This patch add new devfreq_event class for devfreq_event device which provide
> raw data (e.g., memory bus utilization/GPU utilization). This raw data from
> devfreq_event data would be used for the governor of devfreq subsystem.
> - devfreq_event device : Provide raw data for governor of existing devfreq device
> - devfreq device       : Monitor device state and change frequency/voltage of device
>                          using the raw data from devfreq_event device
> 
> The devfreq subsystem support generic DVFS(Dynamic Voltage/Frequency Scaling)
> for Non-CPU Devices. The devfreq device would dertermine current device state
> using various governor (e.g., ondemand, performance, powersave). After completed
> determination of system state, devfreq device would change the frequency/voltage
> of devfreq device according to the result of governor.
> 
> But, devfreq governor must need basic data which indicates current device state.
> Existing devfreq subsystem only consider devfreq device which check current system
> state and determine proper system state using basic data. There is no subsystem
> for device providing basic data to devfreq device.
> 
> The devfreq subsystem must need devfreq_event device(data-provider device) for
> existing devfreq device. So, this patch add new devfreq_event class for
> devfreq_event device which read various basic data(e.g, memory bus utilization,
> GPU utilization) and provide measured data to existing devfreq device through
> standard APIs of devfreq_event class.
> 
> The following description explains the feature of two kind of devfreq class:
> - devfreq class (existing)
>  : devfreq consumer device use raw data from devfreq_event device for
>    determining proper current system state and change voltage/frequency
>    dynamically using various governors.
> 
> - devfreq_event class (new)
>  : Provide measured raw data to devfreq device for governor
> 
> Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
> Cc: Kyungmin Park <kyungmin.park@samsung.com>
> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
> ---
>  drivers/devfreq/Kconfig         |   2 +
>  drivers/devfreq/Makefile        |   5 +-
>  drivers/devfreq/devfreq-event.c | 302 ++++++++++++++++++++++++++++++++++++++++
>  drivers/devfreq/event/Makefile  |   1 +
>  include/linux/devfreq.h         | 141 +++++++++++++++++++
>  5 files changed, 450 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/devfreq/devfreq-event.c
>  create mode 100644 drivers/devfreq/event/Makefile
> 
> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
> index faf4e70..4d15b62 100644
> --- a/drivers/devfreq/Kconfig
> +++ b/drivers/devfreq/Kconfig
> @@ -87,4 +87,6 @@ config ARM_EXYNOS5_BUS_DEVFREQ
>  	  It reads PPMU counters of memory controllers and adjusts the
>  	  operating frequencies and voltages with OPP support.
>  
> +comment "DEVFREQ Event Drivers"
> +
>  endif # PM_DEVFREQ
> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
> index 16138c9..a1ffabe 100644
> --- a/drivers/devfreq/Makefile
> +++ b/drivers/devfreq/Makefile
> @@ -1,4 +1,4 @@
> -obj-$(CONFIG_PM_DEVFREQ)	+= devfreq.o
> +obj-$(CONFIG_PM_DEVFREQ)		+= devfreq.o devfreq-event.o
>  obj-$(CONFIG_DEVFREQ_GOV_SIMPLE_ONDEMAND)	+= governor_simpleondemand.o
>  obj-$(CONFIG_DEVFREQ_GOV_PERFORMANCE)	+= governor_performance.o
>  obj-$(CONFIG_DEVFREQ_GOV_POWERSAVE)	+= governor_powersave.o
> @@ -7,3 +7,6 @@ obj-$(CONFIG_DEVFREQ_GOV_USERSPACE)	+= governor_userspace.o
>  # DEVFREQ Drivers
>  obj-$(CONFIG_ARM_EXYNOS4_BUS_DEVFREQ)	+= exynos/
>  obj-$(CONFIG_ARM_EXYNOS5_BUS_DEVFREQ)	+= exynos/
> +
> +# DEVFREQ Event Drivers
> +obj-$(CONFIG_PM_DEVFREQ)		+= event/
> diff --git a/drivers/devfreq/devfreq-event.c b/drivers/devfreq/devfreq-event.c
> new file mode 100644
> index 0000000..b47329f
> --- /dev/null
> +++ b/drivers/devfreq/devfreq-event.c
> @@ -0,0 +1,302 @@
> +/*
> + * devfreq-event: Generic DEVFREQ Event class driver
> + *
> + * Copyright (C) 2014 Samsung Electronics
> + * Chanwoo Choi <cw00.choi@samsung.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.
> + *
> + * This driver is based on drivers/devfreq/devfreq.c
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/sched.h>
> +#include <linux/errno.h>
> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/stat.h>
> +#include <linux/pm_opp.h>
> +#include <linux/devfreq.h>
> +#include <linux/workqueue.h>
> +#include <linux/platform_device.h>
> +#include <linux/list.h>
> +#include <linux/printk.h>
> +#include <linux/hrtimer.h>
> +#include <linux/of.h>
> +#include "governor.h"
> +
> +static struct class *devfreq_event_class;
> +
> +/* The list of all devfreq event list */
> +static LIST_HEAD(devfreq_event_list);
> +static DEFINE_MUTEX(devfreq_event_list_lock);
> +
> +#define to_devfreq_event(DEV) container_of(DEV, struct devfreq_event_dev, dev)
> +
> +struct devfreq_event_dev *devfreq_add_event_device(struct device *dev,
> +					struct devfreq_event_desc *desc)
> +{
> +	struct devfreq_event_dev *event_dev;
> +	static atomic_t event_no = ATOMIC_INIT(0);
> +	int ret;
> +
> +	if (!dev || !desc)
> +		return ERR_PTR(-EINVAL);
> +
> +	if (!desc->name || !desc->ops)
> +		return ERR_PTR(-EINVAL);
> +
> +	event_dev = kzalloc(sizeof(struct devfreq_event_dev), GFP_KERNEL);
> +	if (!event_dev)
> +		return ERR_PTR(-ENOMEM);

Is this memory freed anywhere when driver is removed? I couldn't find
it. I couldn't also find function like devfreq_remove_event_device()
which would be reverting all the work done when adding.

> +
> +	mutex_lock(&devfreq_event_list_lock);
> +
> +	mutex_init(&event_dev->lock);
> +	event_dev->desc = desc;
> +	event_dev->dev.parent = dev;
> +	event_dev->dev.class = devfreq_event_class;
> +
> +	dev_set_name(&event_dev->dev, "event.%d",
> +		     atomic_inc_return(&event_no) - 1);
> +	ret = device_register(&event_dev->dev);
> +	if (ret != 0) {
> +		put_device(&event_dev->dev);
> +		goto err;
> +	}
> +	dev_set_drvdata(&event_dev->dev, event_dev);
> +
> +	/* Add devfreq event device to devfreq_event_list */
> +	INIT_LIST_HEAD(&event_dev->node);
> +	list_add(&event_dev->node, &devfreq_event_list);
> +
> +	mutex_unlock(&devfreq_event_list_lock);
> +
> +	return event_dev;
> +err:

Missing 'mutex_unlock(&devfreq_event_list_lock)' here.


> +	kfree(event_dev);
> +	return ERR_PTR(ret);
> +}
> +EXPORT_SYMBOL(devfreq_add_event_device);
> +
> +struct devfreq_event_dev *devfreq_get_event_dev(const char *event_dev_name)
> +{
> +	struct devfreq_event_dev *event_dev;
> +
> +	mutex_lock(&devfreq_event_list_lock);
> +	list_for_each_entry(event_dev, &devfreq_event_list, node) {
> +		if (!strcmp(event_dev->desc->name, event_dev_name))
> +			goto out;
> +	}
> +	event_dev = NULL;
> +out:
> +	mutex_unlock(&devfreq_event_list_lock);
> +
> +	return event_dev;
> +}
> +EXPORT_SYMBOL_GPL(devfreq_get_event_dev);
> +
> +struct devfreq_event_dev *devfreq_get_event_dev_by_phandle(struct device *dev,
> +						      int index)
> +{
> +	struct device_node *node;
> +	struct devfreq_event_dev *event_dev;
> +
> +	if (!dev->of_node) {
> +		dev_err(dev, "device does not have a device node entry\n");
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	node = of_parse_phandle(dev->of_node, "devfreq-events", index);
> +	if (!node) {
> +		dev_err(dev, "failed to get phandle in %s node\n",
> +			dev->of_node->full_name);
> +		return ERR_PTR(-ENODEV);
> +	}
> +
> +	event_dev = devfreq_get_event_dev(node->name);
> +	if (!event_dev) {
> +		dev_err(dev, "unable to get devfreq-event device : %s\n",
> +			node->name);

of_node_put() for node obtained with of_parse_phandle().

> +		return ERR_PTR(-ENODEV);
> +	}
> +
> +	return event_dev;
> +}
> +EXPORT_SYMBOL_GPL(devfreq_get_event_dev_by_phandle);
> +
> +int devfreq_put_event_dev(struct devfreq_event_dev *event_dev)
> +{

of_node_put() here to decrement refcnt from of_parse_phandle()?

> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(devfreq_put_event_dev);
> +
> +int devfreq_enable_event_dev(struct devfreq_event_dev *event_dev)
> +{
> +	int ret = 0;
> +
> +	if (!event_dev || !event_dev->desc)
> +		return -EINVAL;
> +
> +	mutex_lock(&event_dev->lock);
> +	if (event_dev->desc->ops && event_dev->desc->ops->enable) {
> +		ret = event_dev->desc->ops->enable(event_dev);
> +		if (ret < 0)
> +			goto err;
> +	}
> +	event_dev->enable_count++;
> +err:
> +	mutex_unlock(&event_dev->lock);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(devfreq_enable_event_dev);
> +
> +int devfreq_disable_event_dev(struct devfreq_event_dev *event_dev)
> +{
> +	int ret = 0;
> +
> +	if (!event_dev || !event_dev->desc)
> +		return -EINVAL;
> +
> +	mutex_lock(&event_dev->lock);
> +	if (event_dev->enable_count > 0) {
> +		event_dev->enable_count--;
> +	} else {
> +		dev_warn(&event_dev->dev, "unbalanced enable_count\n");
> +		ret = -EINVAL;
> +		goto err;
> +	}
> +
> +	if (event_dev->desc->ops && event_dev->desc->ops->disable) {
> +		ret = event_dev->desc->ops->disable(event_dev);
> +		if (ret < 0) {
> +			event_dev->enable_count++;
> +			goto err;
> +		}
> +	}
> +err:
> +	mutex_unlock(&event_dev->lock);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(devfreq_disable_event_dev);
> +
> +bool devfreq_is_enabled_event_dev(struct devfreq_event_dev *event_dev)
> +{
> +	bool enabled = false;
> +
> +	if (!event_dev || !event_dev->desc)
> +		return enabled;
> +
> +	if (!(event_dev->enable_count > 0))
> +		return enabled;
> +
> +	if (event_dev->desc->ops && event_dev->desc->ops->is_enabled)
> +		enabled = event_dev->desc->ops->is_enabled(event_dev);
> +
> +	return enabled;
> +}
> +EXPORT_SYMBOL_GPL(devfreq_is_enabled_event_dev);
> +
> +int devfreq_set_event_event_dev(struct devfreq_event_dev *event_dev)

The convention of names you use is not obvious to me. I would expect
rather devfreq_event_dev_XXX (where XXX is "set_event", "is_enabled"
etc).
The one above is good example what is the issue with current convention:
devfreq_set_event_event_dev
            ^     ^
This double "event" looks weird.

> +{
> +	if (!event_dev || !event_dev->desc)
> +		return -EINVAL;
> +
> +	if (!devfreq_is_enabled_event_dev(event_dev))
> +		return -EPERM;
> +
> +	if (event_dev->desc->ops && event_dev->desc->ops->set_event)
> +		return event_dev->desc->ops->set_event(event_dev);

No mutexes here? What exactly is protected by mutex?

> +
> +	return -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(devfreq_set_event_event_dev);
> +
> +int devfreq_get_event_event_dev(struct devfreq_event_dev *event_dev,
> +				int *total_event)
> +{
> +	if (!event_dev || !event_dev->desc)
> +		return -EINVAL;
> +
> +	if (!devfreq_is_enabled_event_dev(event_dev))
> +		return -EPERM;
> +
> +	if (event_dev->desc->ops && event_dev->desc->ops->get_event)
> +		return event_dev->desc->ops->get_event(event_dev, total_event);
> +
> +	return -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(devfreq_get_event_event_dev);
> +
> +int devfreq_reset_event_dev(struct devfreq_event_dev *event_dev)
> +{
> +	if (!event_dev || !event_dev->desc)
> +		return -EINVAL;
> +
> +	if (!devfreq_is_enabled_event_dev(event_dev))
> +		return -EPERM;
> +
> +	if (event_dev->desc->ops && event_dev->desc->ops->reset)
> +		return event_dev->desc->ops->reset(event_dev);

Same here, no mutex?
> +
> +	return -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(devfreq_reset_event_dev);
> +
> +void *event_dev_get_drvdata(struct devfreq_event_dev *event_dev)
> +{
> +	return event_dev->desc->driver_data;
> +}
> +EXPORT_SYMBOL_GPL(event_dev_get_drvdata);

Best regards,
Krzysztof

Re: [RFC PATCHv2 4/7] devfreq: event: Add exynos-ppmu devfreq event driver

[Krzysztof Kozlowski]

On wto, 2014-12-09 at 23:13 +0900, Chanwoo Choi wrote:
> This patch add exynos-ppmu devfreq event driver to provider raw data about
> the utilization of each IP in Exynos SoC series.
> 
> Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
> Cc: Kyungmin Park <kyungmin.park@samsung.com>
> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
> ---
>  drivers/devfreq/Kconfig             |   9 +
>  drivers/devfreq/event/Makefile      |   1 +
>  drivers/devfreq/event/exynos-ppmu.c | 409 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 419 insertions(+)
>  create mode 100644 drivers/devfreq/event/exynos-ppmu.c

I would rather see this as an incremental change for existing
exynos_ppmu.c file. However I do not insists on that.

> 
> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
> index 4d15b62..d4559f7 100644
> --- a/drivers/devfreq/Kconfig
> +++ b/drivers/devfreq/Kconfig
> @@ -89,4 +89,13 @@ config ARM_EXYNOS5_BUS_DEVFREQ
>  
>  comment "DEVFREQ Event Drivers"
>  
> +config DEVFREQ_EVENT_EXYNOS_PPMU
> +	bool "EXYNOS PPMU (Performance Profiling Monitoring Unit) DEVFREQ event Driver"
> +	depends on ARCH_EXYNOS
> +	select PM_OPP
> +	help
> +	 This add the DEVFREQ event driver for Exynos SoC. It provides PPMU
> +	 (Performance Profiling Monitoring Unit) counters to estimate the
> +	 utilization of each module.
> +
>  endif # PM_DEVFREQ
> diff --git a/drivers/devfreq/event/Makefile b/drivers/devfreq/event/Makefile
> index dc56005..be146ea 100644
> --- a/drivers/devfreq/event/Makefile
> +++ b/drivers/devfreq/event/Makefile
> @@ -1 +1,2 @@
>  # Exynos DEVFREQ Event Drivers
> +obj-$(CONFIG_DEVFREQ_EVENT_EXYNOS_PPMU) += exynos-ppmu.o
> diff --git a/drivers/devfreq/event/exynos-ppmu.c b/drivers/devfreq/event/exynos-ppmu.c
> new file mode 100644
> index 0000000..2706f23
> --- /dev/null
> +++ b/drivers/devfreq/event/exynos-ppmu.c
> @@ -0,0 +1,409 @@
> +/*
> + * exynos_ppmu.c - EXYNOS PPMU (Performance Profiling Monitoring Units) support
> + *
> + * Copyright (c) 2014 Samsung Electronics Co., Ltd.
> + * Author : Chanwoo Choi <cw00.choi@samsung.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.
> + *
> + * This driver is based on drivers/devfreq/exynos/exynos_ppmu.c
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of_address.h>
> +#include <linux/platform_device.h>
> +#include <linux/suspend.h>
> +#include <linux/devfreq.h>
> +
> +#define PPMU_ENABLE             BIT(0)
> +#define PPMU_DISABLE            0x0
> +#define PPMU_CYCLE_RESET        BIT(1)
> +#define PPMU_COUNTER_RESET      BIT(2)
> +
> +#define PPMU_ENABLE_COUNT0      BIT(0)
> +#define PPMU_ENABLE_COUNT1      BIT(1)
> +#define PPMU_ENABLE_COUNT2      BIT(2)
> +#define PPMU_ENABLE_COUNT3      BIT(3)
> +#define PPMU_ENABLE_CYCLE       BIT(31)
> +
> +#define PPMU_CNTENS		0x10
> +#define PPMU_FLAG		0x50
> +#define PPMU_CCNT_OVERFLOW	BIT(31)
> +#define PPMU_CCNT		0x100
> +
> +#define PPMU_PMCNT0		0x110
> +#define PPMU_PMCNT_OFFSET	0x10
> +#define PMCNT_OFFSET(x)		(PPMU_PMCNT0 + (PPMU_PMCNT_OFFSET * x))
> +
> +#define PPMU_BEVT0SEL		0x1000
> +#define PPMU_BEVTSEL_OFFSET	0x100
> +#define PPMU_BEVTSEL(x)		(PPMU_BEVT0SEL + (x * PPMU_BEVTSEL_OFFSET))
> +
> +#define RD_DATA_COUNT		0x5
> +#define WR_DATA_COUNT		0x6
> +#define RDWR_DATA_COUNT		0x7
> +
> +enum ppmu_counter {
> +	PPMU_PMNCNT0,
> +	PPMU_PMNCNT1,
> +	PPMU_PMNCNT2,
> +	PPMU_PMNCNT3,
> +	PPMU_PMNCNT_MAX,
> +};
> +
> +/* Platform data */
> +struct exynos_ppmu_data {
> +	struct devfreq *devfreq;

Looks like not used here.

> +	struct devfreq_event_dev **event_dev;
> +	struct devfreq_event_desc *desc;
> +	unsigned int num_events;
> +
> +	struct device *dev;
> +	struct clk *clk_ppmu;
> +	struct mutex lock;
> +
> +	struct __exynos_ppmu {
> +		void __iomem *hw_base;
> +		unsigned int ccnt;
> +		unsigned int event[PPMU_PMNCNT_MAX];
> +		unsigned int count[PPMU_PMNCNT_MAX];
> +		unsigned long long ns;
> +		ktime_t reset_time;
> +		bool ccnt_overflow;
> +		bool count_overflow[PPMU_PMNCNT_MAX];
> +	} ppmu;
> +};
> +
> +struct __exynos_ppmu_events {
> +	char *name;
> +	int id;
> +} ppmu_events[] = {
> +	{ "ppmu-dmc0-pmcnt0", PPMU_PMNCNT0 },
> +	{ "ppmu-dmc0-pmcnt1", PPMU_PMNCNT1 },
> +	{ "ppmu-dmc0-pmcnt2", PPMU_PMNCNT2 },
> +	{ "ppmu-dmc0-pmcnt3", PPMU_PMNCNT3 },
> +
> +	{ "ppmu-dmc1-pmcnt0", PPMU_PMNCNT0 },
> +	{ "ppmu-dmc1-pmcnt1", PPMU_PMNCNT1 },
> +	{ "ppmu-dmc1-pmcnt2", PPMU_PMNCNT2 },
> +	{ "ppmu-dmc1-pmcnt3", PPMU_PMNCNT3 },
> +
> +	{ "ppmu-cpu-pmcnt0", PPMU_PMNCNT0 },
> +	{ "ppmu-cpu-pmcnt1", PPMU_PMNCNT1 },
> +	{ "ppmu-cpu-pmcnt2", PPMU_PMNCNT2 },
> +	{ "ppmu-cpu-pmcnt3", PPMU_PMNCNT3 },
> +
> +	{ "ppmu-rightbus-pmcnt0", PPMU_PMNCNT0 },
> +	{ "ppmu-rightbus-pmcnt1", PPMU_PMNCNT1 },
> +	{ "ppmu-rightbus-pmcnt2", PPMU_PMNCNT2 },
> +	{ "ppmu-rightbus-pmcnt3", PPMU_PMNCNT3 },
> +
> +	{ "ppmu-leftbus-pmcnt0", PPMU_PMNCNT0 },
> +	{ "ppmu-leftbus-pmcnt1", PPMU_PMNCNT1 },
> +	{ "ppmu-leftbus-pmcnt2", PPMU_PMNCNT2 },
> +	{ "ppmu-leftbus-pmcnt3", PPMU_PMNCNT3 },
> +
> +	{ "ppmu-camif-pmcnt0", PPMU_PMNCNT0 },
> +	{ "ppmu-camif-pmcnt1", PPMU_PMNCNT1 },
> +	{ "ppmu-camif-pmcnt2", PPMU_PMNCNT2 },
> +	{ "ppmu-camif-pmcnt3", PPMU_PMNCNT3 },
> +
> +	{ "ppmu-lcd0-pmcnt0", PPMU_PMNCNT0 },
> +	{ "ppmu-lcd0-pmcnt1", PPMU_PMNCNT1 },
> +	{ "ppmu-lcd0-pmcnt2", PPMU_PMNCNT2 },
> +	{ "ppmu-lcd0-pmcnt3", PPMU_PMNCNT3 },
> +
> +	{ "ppmu-g3d-pmcnt0", PPMU_PMNCNT0 },
> +	{ "ppmu-g3d-pmcnt1", PPMU_PMNCNT1 },
> +	{ "ppmu-g3d-pmcnt2", PPMU_PMNCNT2 },
> +	{ "ppmu-g3d-pmcnt3", PPMU_PMNCNT3 },
> +
> +	{ "ppmu-mfc-pmcnt0", PPMU_PMNCNT0 },
> +	{ "ppmu-mfc-pmcnt1", PPMU_PMNCNT1 },
> +	{ "ppmu-mfc-pmcnt2", PPMU_PMNCNT2 },
> +	{ "ppmu-mfc-pmcnt3", PPMU_PMNCNT3 },
> +
> +	{ "ppmu-fsys-pmcnt0", PPMU_PMNCNT0 },
> +	{ "ppmu-fsys-pmcnt1", PPMU_PMNCNT1 },
> +	{ "ppmu-fsys-pmcnt2", PPMU_PMNCNT2 },
> +	{ "ppmu-fsys-pmcnt3", PPMU_PMNCNT3 },
> +	{ /* sentinel */ },
> +};
> +
> +static int exynos_ppmu_enable(struct devfreq_event_dev *event_dev)
> +{
> +	struct exynos_ppmu_data *exynos_ppmu = event_dev_get_drvdata(event_dev);
> +
> +	__raw_writel(PPMU_ENABLE, exynos_ppmu->ppmu.hw_base);
> +
> +	return 0;
> +}
> +
> +static int exynos_ppmu_disable(struct devfreq_event_dev *event_dev)
> +{
> +	struct exynos_ppmu_data *exynos_ppmu = event_dev_get_drvdata(event_dev);
> +
> +	__raw_writel(PPMU_DISABLE, exynos_ppmu->ppmu.hw_base);
> +
> +	return 0;
> +}
> +
> +static bool exynos_ppmu_is_enabled(struct devfreq_event_dev *event_dev)
> +{
> +	struct exynos_ppmu_data *exynos_ppmu = event_dev_get_drvdata(event_dev);
> +	int val;
> +
> +	val = __raw_readl(exynos_ppmu->ppmu.hw_base);
> +	if (val & PPMU_ENABLE)
> +		return true;
> +
> +	return false;
> +}
> +
> +static int exynos_ppmu_find_ppmu_id(struct devfreq_event_dev *event_dev)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(ppmu_events); i++)
> +		if (!strcmp(event_dev->desc->name, ppmu_events[i].name))
> +			return ppmu_events[i].id;
> +
> +	return -EINVAL;
> +}
> +
> +static int exynos_ppmu_set_event(struct devfreq_event_dev *event_dev,
> +				enum devfreq_event_type event_type)
> +{
> +	struct exynos_ppmu_data *exynos_ppmu = event_dev_get_drvdata(event_dev);
> +	void __iomem *ppmu_base = exynos_ppmu->ppmu.hw_base;
> +	int id = exynos_ppmu_find_ppmu_id(event_dev);
> +
> +	__raw_writel(RDWR_DATA_COUNT, ppmu_base + PPMU_BEVTSEL(id));
> +
> +	return 0;
> +}
> +
> +static int exynos_ppmu_get_event(struct devfreq_event_dev *event_dev,
> +				enum devfreq_event_type event_type,
> +				int *total_event)
> +{
> +	struct exynos_ppmu_data *exynos_ppmu = event_dev_get_drvdata(event_dev);
> +	void __iomem *ppmu_base = exynos_ppmu->ppmu.hw_base;
> +	int id = exynos_ppmu_find_ppmu_id(event_dev);
> +	int cnt, total_cnt;
> +
> +	total_cnt = __raw_readl(ppmu_base + PPMU_CCNT);
> +
> +	if (id == PPMU_PMNCNT3)
> +		cnt = ((__raw_readl(ppmu_base + PMCNT_OFFSET(id)) << 8) |
> +			  __raw_readl(ppmu_base + PMCNT_OFFSET(id + 1)));
> +	else
> +		cnt = __raw_readl(ppmu_base + PMCNT_OFFSET(id));
> +
> +	*total_event = total_cnt;
> +
> +	return cnt;
> +}
> +
> +static int exynos_ppmu_reset(struct devfreq_event_dev *event_dev)
> +{
> +	struct exynos_ppmu_data *exynos_ppmu = event_dev_get_drvdata(event_dev);
> +	void __iomem *ppmu_base = exynos_ppmu->ppmu.hw_base;
> +
> +	__raw_writel(PPMU_CYCLE_RESET | PPMU_COUNTER_RESET, ppmu_base);
> +	__raw_writel(PPMU_ENABLE_CYCLE  |
> +		     PPMU_ENABLE_COUNT0 |
> +		     PPMU_ENABLE_COUNT1 |
> +		     PPMU_ENABLE_COUNT2 |
> +		     PPMU_ENABLE_COUNT3,
> +		     ppmu_base + PPMU_CNTENS);
> +	__raw_writel(PPMU_ENABLE, exynos_ppmu->ppmu.hw_base);
> +
> +	return 0;
> +}
> +
> +static struct devfreq_event_ops exynos_ppmu_ops = {
> +	.enable		= exynos_ppmu_enable,
> +	.disable	= exynos_ppmu_disable,
> +	.is_enabled	= exynos_ppmu_is_enabled,
> +	.set_event	= exynos_ppmu_set_event,
> +	.get_event	= exynos_ppmu_get_event,
> +	.reset		= exynos_ppmu_reset,
> +};
> +
> +static int of_get_devfreq_events(struct device_node *np,
> +				struct exynos_ppmu_data *exynos_ppmu)
> +{
> +	struct devfreq_event_desc *desc;
> +	struct device *dev = exynos_ppmu->dev;
> +	struct device_node *events_np, *node;
> +	int i, j, count;
> +
> +	events_np = of_find_node_by_name(np, "events");

of_get_child_by_name()

> +	if (!events_np) {
> +		dev_err(dev, "Failed to find ppmus sub-node\n");
> +		return -EINVAL;
> +	}
> +
> +	count = of_get_child_count(events_np);
> +	desc = devm_kzalloc(dev, sizeof(struct devfreq_event_desc) * count,
> +			      GFP_KERNEL);
> +	if (!desc)
> +		return -ENOMEM;
> +	exynos_ppmu->num_events = count;
> +
> +	j = 0;
> +	for_each_child_of_node(events_np, node) {
> +		for (i = 0; i < ARRAY_SIZE(ppmu_events); i++) {
> +			if (!of_node_cmp(node->name, ppmu_events[i].name))
> +				break;
> +		}
> +
> +		if (i == ARRAY_SIZE(ppmu_events)) {
> +			dev_warn(dev,
> +				"don't know how to configure events : %s\n",
> +				node->name);
> +			continue;
> +		}
> +		desc[j].ops = &exynos_ppmu_ops;
> +		desc[j].driver_data = exynos_ppmu;
> +		desc[j].event_type |= DEVFREQ_EVENT_TYPE_RAW_DATA;
> +		of_property_read_string(node, "event-name", &desc[j].name);
> +		j++;
> +	}
> +	exynos_ppmu->desc = desc;

of_node_put() for 'events_np' and 'node' in loop.

> +
> +	return 0;
> +}
> +
> +static int exynos_ppmu_parse_dt(struct exynos_ppmu_data *exynos_ppmu)
> +{
> +	struct device *dev = exynos_ppmu->dev;
> +	struct device_node *np = dev->of_node;
> +	int ret = 0;
> +
> +	if (!np) {
> +		dev_err(dev, "Failed to find devicetree node\n");
> +		return -EINVAL;
> +	}
> +
> +	/* Maps the memory mapped IO to control PPMU register */
> +	exynos_ppmu->ppmu.hw_base = of_iomap(np, 0);
> +	if (IS_ERR_OR_NULL(exynos_ppmu->ppmu.hw_base)) {
> +		dev_err(dev, "Failed to map memory region\n");
> +		ret = -EINVAL;
> +		goto err_iomap;
> +	}
> +
> +	/* FIXME: Get the clock of ppmu and enable this clock */
> +	exynos_ppmu->clk_ppmu = devm_clk_get(dev, "ppmu");
> +	if (IS_ERR(exynos_ppmu->clk_ppmu))
> +		dev_warn(dev, "Failed to get PPMU clock\n");
> +
> +	ret = of_get_devfreq_events(np, exynos_ppmu);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to parse exynos ppmu dt node\n");
> +		goto err_clock;
> +	}
> +
> +	return 0;
> +
> +err_clock:
> +	clk_disable_unprepare(exynos_ppmu->clk_ppmu);

Not needed. Clock is not enabled here.

> +err_iomap:
> +	iounmap(exynos_ppmu->ppmu.hw_base);
> +
> +	return ret;
> +}
> +
> +static int exynos_ppmu_probe(struct platform_device *pdev)
> +{
> +	struct exynos_ppmu_data *exynos_ppmu;
> +	struct devfreq_event_dev **event_dev;
> +	struct devfreq_event_desc *desc;
> +	int i, ret = 0, size;
> +
> +	/* Allocate the memory of exynos_ppmu_data and initialize it */
> +	exynos_ppmu = devm_kzalloc(&pdev->dev, sizeof(struct exynos_ppmu_data),
> +				   GFP_KERNEL);
> +	if (!exynos_ppmu)
> +		return -ENOMEM;
> +
> +	mutex_init(&exynos_ppmu->lock);
> +	exynos_ppmu->dev = &pdev->dev;
> +
> +	/* Parse dt data to get resource */
> +	ret = exynos_ppmu_parse_dt(exynos_ppmu);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "Failed to parse DT node for resource\n");
> +		return ret;
> +	}
> +	desc = exynos_ppmu->desc;
> +
> +	size = sizeof(struct devfreq_event_dev *) * exynos_ppmu->num_events;
> +	exynos_ppmu->event_dev = devm_kzalloc(&pdev->dev, size, GFP_KERNEL);
> +	if (!exynos_ppmu->event_dev) {
> +		dev_err(&pdev->dev,
> +			"Failed to allocate memory devfreq_event_dev list\n");
> +		return -ENOMEM;
> +	}
> +	event_dev = exynos_ppmu->event_dev;
> +	platform_set_drvdata(pdev, exynos_ppmu);

Missing clk_prepare_enable().

> +
> +	for (i = 0; i < exynos_ppmu->num_events; i++) {
> +		event_dev[i] = devfreq_add_event_device(&pdev->dev, &desc[i]);
> +		if (IS_ERR(event_dev)) {
> +			ret = PTR_ERR(event_dev);
> +			dev_err(&pdev->dev, "Failed to add devfreq evt dev\n");
> +			goto err;
> +		}
> +	}
> +
> +	return 0;
> +err:
> +	clk_disable_unprepare(exynos_ppmu->clk_ppmu);
> +	iounmap(exynos_ppmu->ppmu.hw_base);
> +
> +	return ret;
> +}
> +
> +static int exynos_ppmu_remove(struct platform_device *pdev)
> +{
> +	struct exynos_ppmu_data *exynos_ppmu = platform_get_drvdata(pdev);
> +
> +	clk_disable_unprepare(exynos_ppmu->clk_ppmu);
> +	iounmap(exynos_ppmu->ppmu.hw_base);
> +
> +	/* Remove devfreq instance */
> +	devfreq_remove_device(exynos_ppmu->devfreq);

Shouldn't this be devfreq_remove_event_dev()?

Best regards,
Krzysztof

Re: [RFC PATCHv2 0/7] devfreq: Add devfreq-event class to provide raw data for devfreq device

[Krzysztof Kozlowski]

On wto, 2014-12-09 at 23:12 +0900, Chanwoo Choi wrote:
> This patchset add new devfreq_event class to provide raw data to determine
> current utilization of device  which is used for devfreq governor.
> 
> [Description of devfreq-event class]
> This patchset add new devfreq_event class for devfreq_event device which provide
> raw data (e.g., memory bus utilization/GPU utilization). This raw data from
> devfreq_event data would be used for the governor of devfreq subsystem.
> - devfreq_event device : Provide raw data for governor of existing devfreq device
> - devfreq device       : Monitor device state and change frequency/voltage of device
>                          using the raw data from devfreq_event device
> 
> The devfreq subsystem support generic DVFS(Dynamic Voltage/Frequency Scaling)
> for Non-CPU Devices. The devfreq device would dertermine current device state
> using various governor (e.g., ondemand, performance, powersave). After completed
> determination of system state, devfreq device would change the frequency/voltage
> of devfreq device according to the result of governor.
> 
> But, devfreq governor must need basic data which indicates current device state.
> Existing devfreq subsystem only consider devfreq device which check current system
> state and determine proper system state using basic data. There is no subsystem
> for device providing basic data to devfreq device.
> 
> The devfreq subsystem must need devfreq_event device(data-provider device) for
> existing devfreq device. So, this patch add new devfreq_event class for
> devfreq_event device which read various basic data(e.g, memory bus utilization,
> GPU utilization) and provide measured data to existing devfreq device through
> standard APIs of devfreq_event class.
> 
> The following description explains the feature of two kind of devfreq class:
> - devfreq class (existing)
>  : devfreq consumer device use raw data from devfreq_event device for
>    determining proper current system state and change voltage/frequency
>    dynamically using various governors.
> - devfreq_event class (new)
>  : Provide measured raw data to devfreq device for governor
> 
> Also, the devfreq-event device would support various type event as following:
>  : DEVFREQ_EVENT_TYPE_RAW_DATA
>  : DEVFREQ_EVENT_TYPE_UTILIZATION
>  : DEVFREQ_EVENT_TYPE_BANDWIDTH
>  : DEVFREQ_EVENT_TYPE_LATENCY
> 
> [For example]
> If board dts includes PPMU_DMC0/DMC1/CPU event node,
> would show following sysfs entry. Also devfreq driver(e.g., exynos4_bus.c)
> can get the instance of devfreq-event device by using provided API and then
> get raw data which reflect the current state of device.
> 

Hi,

Overall I like the idea. I understand that now devfreq devices (like
exynos devfreq) should bind themselves to buses, not to PPMU. It makes
sense to me because bus clock and bus voltage are properties of bus, not
monitoring unit.

I see that this is still a RFC so it would be hard to base current
devfreq work on top of it.

One more general comment: you're adding a some API which is not used by
current devfreq_event user (exynos). For example the exclusive flag or
event types. I think it will be simpler to stick to the basic approach
(reduced API). If some other user emerges then new API will be added.

Best regards,
Krzysztof


> -sh-3.2# pwd
> /sys/class/devfreq_event
> -sh-3.2# ls -al
> total 0
> drwxr-xr-x  2 root root 0 Jan  7 11:10 .
> drwxr-xr-x 37 root root 0 Jan  7 11:10 ..
> lrwxrwxrwx  1 root root 0 Jan  7 11:10 event.0 -> ../../devices/soc/106a0000.ppmu_dmc0/devfreq_event/event.0
> lrwxrwxrwx  1 root root 0 Jan  7 11:10 event.1 -> ../../devices/soc/106a0000.ppmu_dmc0/devfreq_event/event.1
> lrwxrwxrwx  1 root root 0 Jan  7 11:10 event.2 -> ../../devices/soc/106a0000.ppmu_dmc0/devfreq_event/event.2
> lrwxrwxrwx  1 root root 0 Jan  7 11:10 event.3 -> ../../devices/soc/106a0000.ppmu_dmc0/devfreq_event/event.3
> lrwxrwxrwx  1 root root 0 Jan  7 11:10 event.4 -> ../../devices/soc/106b0000.ppmu_dmc1/devfreq_event/event.4
> lrwxrwxrwx  1 root root 0 Jan  7 11:10 event.5 -> ../../devices/soc/106c0000.ppmu_cpu/devfreq_event/event.5
> lrwxrwxrwx  1 root root 0 Jan  7 11:10 event.6 -> ../../devices/soc/106c0000.ppmu_cpu/devfreq_event/event.6
> lrwxrwxrwx  1 root root 0 Jan  7 11:10 event.7 -> ../../devices/soc/106c0000.ppmu_cpu/devfreq_event/event.7
> lrwxrwxrwx  1 root root 0 Jan  7 11:10 event.8 -> ../../devices/soc/106c0000.ppmu_cpu/devfreq_event/event.8
> 
> Changes from v1:
> - Code clean
> - Add the description of devfreq-event structure
> - Add 'is_enabled' function to devfreq_event_ops structure
> - Add 'enable_count' field to devfreq_event_dev structure
> - Check whether devfreq-event device is enabled or not
>   during calling devfreq_event API
> - Define the type of devfreq-event device as following
>   : DEVFREQ_EVENT_TYPE_RAW_DATA
>   : DEVFREQ_EVENT_TYPE_UTILIZATION
>   : DEVFREQ_EVENT_TYPE_BANDWIDTH
>   : DEVFREQ_EVENT_TYPE_LATENCY
> - Add the exclusive feature of devfreq-event device.
>   If devfreq-event device is used on only on devfreq driver,
>   should used 'devfreq_enable_event_dev_exclusive()' function
> - Add new patch6 for test on Exynos3250-based Rinato board
> 
> Chanwoo Choi (7):
>   devfreq: event: Add new devfreq_event class to provide basic data for devfreq governor
>   devfreq: event: Add the list of supported devfreq-event type
>   devfreq: event: Add the exclusive flag for devfreq-event device
>   devfreq: event: Add exynos-ppmu devfreq event driver
>   ARM: dts: Add PPMU dt node for Exynos3250
>   ARM: dts: Add PPMU dt node for Exynos4 SoC
>   ARM: dts: exynos: Add PPMU dt node to Exynos3250-based Rinato board
> 
>  arch/arm/boot/dts/exynos3250-rinato.dts |  55 +++++
>  arch/arm/boot/dts/exynos3250.dtsi       |  66 ++++++
>  arch/arm/boot/dts/exynos4.dtsi          |  96 ++++++++
>  arch/arm/boot/dts/exynos4210.dtsi       |   8 +
>  drivers/devfreq/Kconfig                 |  11 +
>  drivers/devfreq/Makefile                |   5 +-
>  drivers/devfreq/devfreq-event.c         | 363 ++++++++++++++++++++++++++++
>  drivers/devfreq/event/Makefile          |   2 +
>  drivers/devfreq/event/exynos-ppmu.c     | 409 ++++++++++++++++++++++++++++++++
>  include/linux/devfreq.h                 | 178 ++++++++++++++
>  10 files changed, 1192 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/devfreq/devfreq-event.c
>  create mode 100644 drivers/devfreq/event/Makefile
>  create mode 100644 drivers/devfreq/event/exynos-ppmu.c
> 

Re: [RFC PATCHv2 0/7] devfreq: Add devfreq-event class to provide raw data for devfreq device

[Chanwoo Choi]

Hi Krzysztof,

On Wed, Dec 10, 2014 at 7:07 PM, Krzysztof Kozlowski
<k.kozlowski@samsung.com> wrote:
> On wto, 2014-12-09 at 23:12 +0900, Chanwoo Choi wrote:
>> This patchset add new devfreq_event class to provide raw data to determine
>> current utilization of device  which is used for devfreq governor.
>>
>> [Description of devfreq-event class]
>> This patchset add new devfreq_event class for devfreq_event device which provide
>> raw data (e.g., memory bus utilization/GPU utilization). This raw data from
>> devfreq_event data would be used for the governor of devfreq subsystem.
>> - devfreq_event device : Provide raw data for governor of existing devfreq device
>> - devfreq device       : Monitor device state and change frequency/voltage of device
>>                          using the raw data from devfreq_event device
>>
>> The devfreq subsystem support generic DVFS(Dynamic Voltage/Frequency Scaling)
>> for Non-CPU Devices. The devfreq device would dertermine current device state
>> using various governor (e.g., ondemand, performance, powersave). After completed
>> determination of system state, devfreq device would change the frequency/voltage
>> of devfreq device according to the result of governor.
>>
>> But, devfreq governor must need basic data which indicates current device state.
>> Existing devfreq subsystem only consider devfreq device which check current system
>> state and determine proper system state using basic data. There is no subsystem
>> for device providing basic data to devfreq device.
>>
>> The devfreq subsystem must need devfreq_event device(data-provider device) for
>> existing devfreq device. So, this patch add new devfreq_event class for
>> devfreq_event device which read various basic data(e.g, memory bus utilization,
>> GPU utilization) and provide measured data to existing devfreq device through
>> standard APIs of devfreq_event class.
>>
>> The following description explains the feature of two kind of devfreq class:
>> - devfreq class (existing)
>>  : devfreq consumer device use raw data from devfreq_event device for
>>    determining proper current system state and change voltage/frequency
>>    dynamically using various governors.
>> - devfreq_event class (new)
>>  : Provide measured raw data to devfreq device for governor
>>
>> Also, the devfreq-event device would support various type event as following:
>>  : DEVFREQ_EVENT_TYPE_RAW_DATA
>>  : DEVFREQ_EVENT_TYPE_UTILIZATION
>>  : DEVFREQ_EVENT_TYPE_BANDWIDTH
>>  : DEVFREQ_EVENT_TYPE_LATENCY
>>
>> [For example]
>> If board dts includes PPMU_DMC0/DMC1/CPU event node,
>> would show following sysfs entry. Also devfreq driver(e.g., exynos4_bus.c)
>> can get the instance of devfreq-event device by using provided API and then
>> get raw data which reflect the current state of device.
>>
>
> Hi,
>
> Overall I like the idea. I understand that now devfreq devices (like
> exynos devfreq) should bind themselves to buses, not to PPMU. It makes
> sense to me because bus clock and bus voltage are properties of bus, not
> monitoring unit.
>
> I see that this is still a RFC so it would be hard to base current
> devfreq work on top of it.

The goal of this patch set is not for devfreq device. This patch set
just is used for
devfreq-event device according to patch description.

The devfreq must need devfreq-event device (e.g., exynos-ppmu.c)
but current devfreq subsystem have not been supported any reason for
exyons-ppmu.c as devfreq-event device.

I discussed this same issue on following patch[1] which was posted to support
exynos4 busfreq by me.
[1] https://lkml.org/lkml/2014/3/14/132
- [PATCHv2 8/8] devfreq: exynos4: Add busfreq driver for exynos4210/exynos4x12
After discussion, I thought that devfreq-event class must be needed
before supporting
the dt of devfreq device.

So, I think that devfreq have to support the devfreq-event subsystem
for exynos-ppmu. And then I'll work to support the device-tree of devfreq device
as next job.

>
> One more general comment: you're adding a some API which is not used by
> current devfreq_event user (exynos). For example the exclusive flag or
> event types. I think it will be simpler to stick to the basic approach
> (reduced API). If some other user emerges then new API will be added.

I think that devfreq-event must support both event_flag(e.g., exclusive flag)
and event_type(e.g., raw_data, utilization, bandwidth, latency) because
devfreq-event device (e.g., exynos-ppmu) would provide various event data.
For example, exynos-ppmu driver could support the utilization/bandwidth/latency
of each IP of Exynos SoC. I checked PPMU IP on Exynos TRM.

Also, If specific devfreq-event device should be used on only one device driver,
devfrewq-event class have to support 'exclusive' flag. If
devfreq-event class don't
support the exclusive flag, devfreq-event could not guarantee the
integrity of event
from devfreq-event device.

Thanks for your comment.

Best Regards,
Chanwoo Choi

Re: [RFC PATCHv2 1/7] devfreq: event: Add new devfreq_event class to provide basic data for devfreq governor

[Chanwoo Choi]

Hi Krzysztof,

First of all, thanks for your review.

On 12/10/2014 06:37 PM, Krzysztof Kozlowski wrote:
> On wto, 2014-12-09 at 23:13 +0900, Chanwoo Choi wrote:
>> This patch add new devfreq_event class for devfreq_event device which provide
>> raw data (e.g., memory bus utilization/GPU utilization). This raw data from
>> devfreq_event data would be used for the governor of devfreq subsystem.
>> - devfreq_event device : Provide raw data for governor of existing devfreq device
>> - devfreq device       : Monitor device state and change frequency/voltage of device
>>                          using the raw data from devfreq_event device
>>
>> The devfreq subsystem support generic DVFS(Dynamic Voltage/Frequency Scaling)
>> for Non-CPU Devices. The devfreq device would dertermine current device state
>> using various governor (e.g., ondemand, performance, powersave). After completed
>> determination of system state, devfreq device would change the frequency/voltage
>> of devfreq device according to the result of governor.
>>
>> But, devfreq governor must need basic data which indicates current device state.
>> Existing devfreq subsystem only consider devfreq device which check current system
>> state and determine proper system state using basic data. There is no subsystem
>> for device providing basic data to devfreq device.
>>
>> The devfreq subsystem must need devfreq_event device(data-provider device) for
>> existing devfreq device. So, this patch add new devfreq_event class for
>> devfreq_event device which read various basic data(e.g, memory bus utilization,
>> GPU utilization) and provide measured data to existing devfreq device through
>> standard APIs of devfreq_event class.
>>
>> The following description explains the feature of two kind of devfreq class:
>> - devfreq class (existing)
>>  : devfreq consumer device use raw data from devfreq_event device for
>>    determining proper current system state and change voltage/frequency
>>    dynamically using various governors.
>>
>> - devfreq_event class (new)
>>  : Provide measured raw data to devfreq device for governor
>>
>> Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
>> Cc: Kyungmin Park <kyungmin.park@samsung.com>
>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>> ---
>>  drivers/devfreq/Kconfig         |   2 +
>>  drivers/devfreq/Makefile        |   5 +-
>>  drivers/devfreq/devfreq-event.c | 302 ++++++++++++++++++++++++++++++++++++++++
>>  drivers/devfreq/event/Makefile  |   1 +
>>  include/linux/devfreq.h         | 141 +++++++++++++++++++
>>  5 files changed, 450 insertions(+), 1 deletion(-)
>>  create mode 100644 drivers/devfreq/devfreq-event.c
>>  create mode 100644 drivers/devfreq/event/Makefile
>>
>> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
>> index faf4e70..4d15b62 100644
>> --- a/drivers/devfreq/Kconfig
>> +++ b/drivers/devfreq/Kconfig
>> @@ -87,4 +87,6 @@ config ARM_EXYNOS5_BUS_DEVFREQ
>>  	  It reads PPMU counters of memory controllers and adjusts the
>>  	  operating frequencies and voltages with OPP support.
>>  
>> +comment "DEVFREQ Event Drivers"
>> +
>>  endif # PM_DEVFREQ
>> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
>> index 16138c9..a1ffabe 100644
>> --- a/drivers/devfreq/Makefile
>> +++ b/drivers/devfreq/Makefile
>> @@ -1,4 +1,4 @@
>> -obj-$(CONFIG_PM_DEVFREQ)	+= devfreq.o
>> +obj-$(CONFIG_PM_DEVFREQ)		+= devfreq.o devfreq-event.o
>>  obj-$(CONFIG_DEVFREQ_GOV_SIMPLE_ONDEMAND)	+= governor_simpleondemand.o
>>  obj-$(CONFIG_DEVFREQ_GOV_PERFORMANCE)	+= governor_performance.o
>>  obj-$(CONFIG_DEVFREQ_GOV_POWERSAVE)	+= governor_powersave.o
>> @@ -7,3 +7,6 @@ obj-$(CONFIG_DEVFREQ_GOV_USERSPACE)	+= governor_userspace.o
>>  # DEVFREQ Drivers
>>  obj-$(CONFIG_ARM_EXYNOS4_BUS_DEVFREQ)	+= exynos/
>>  obj-$(CONFIG_ARM_EXYNOS5_BUS_DEVFREQ)	+= exynos/
>> +
>> +# DEVFREQ Event Drivers
>> +obj-$(CONFIG_PM_DEVFREQ)		+= event/
>> diff --git a/drivers/devfreq/devfreq-event.c b/drivers/devfreq/devfreq-event.c
>> new file mode 100644
>> index 0000000..b47329f
>> --- /dev/null
>> +++ b/drivers/devfreq/devfreq-event.c
>> @@ -0,0 +1,302 @@
>> +/*
>> + * devfreq-event: Generic DEVFREQ Event class driver
>> + *
>> + * Copyright (C) 2014 Samsung Electronics
>> + * Chanwoo Choi <cw00.choi@samsung.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.
>> + *
>> + * This driver is based on drivers/devfreq/devfreq.c
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/sched.h>
>> +#include <linux/errno.h>
>> +#include <linux/err.h>
>> +#include <linux/init.h>
>> +#include <linux/module.h>
>> +#include <linux/slab.h>
>> +#include <linux/stat.h>
>> +#include <linux/pm_opp.h>
>> +#include <linux/devfreq.h>
>> +#include <linux/workqueue.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/list.h>
>> +#include <linux/printk.h>
>> +#include <linux/hrtimer.h>
>> +#include <linux/of.h>
>> +#include "governor.h"
>> +
>> +static struct class *devfreq_event_class;
>> +
>> +/* The list of all devfreq event list */
>> +static LIST_HEAD(devfreq_event_list);
>> +static DEFINE_MUTEX(devfreq_event_list_lock);
>> +
>> +#define to_devfreq_event(DEV) container_of(DEV, struct devfreq_event_dev, dev)
>> +
>> +struct devfreq_event_dev *devfreq_add_event_device(struct device *dev,
>> +					struct devfreq_event_desc *desc)
>> +{
>> +	struct devfreq_event_dev *event_dev;
>> +	static atomic_t event_no = ATOMIC_INIT(0);
>> +	int ret;
>> +
>> +	if (!dev || !desc)
>> +		return ERR_PTR(-EINVAL);
>> +
>> +	if (!desc->name || !desc->ops)
>> +		return ERR_PTR(-EINVAL);
>> +
>> +	event_dev = kzalloc(sizeof(struct devfreq_event_dev), GFP_KERNEL);
>> +	if (!event_dev)
>> +		return ERR_PTR(-ENOMEM);
> 
> Is this memory freed anywhere when driver is removed? I couldn't find
> it. I couldn't also find function like devfreq_remove_event_device()
> which would be reverting all the work done when adding.

You're right. I'll use devm_kzalloc instead of kzalloc.

> 
>> +
>> +	mutex_lock(&devfreq_event_list_lock);
>> +
>> +	mutex_init(&event_dev->lock);
>> +	event_dev->desc = desc;
>> +	event_dev->dev.parent = dev;
>> +	event_dev->dev.class = devfreq_event_class;
>> +
>> +	dev_set_name(&event_dev->dev, "event.%d",
>> +		     atomic_inc_return(&event_no) - 1);
>> +	ret = device_register(&event_dev->dev);
>> +	if (ret != 0) {
>> +		put_device(&event_dev->dev);
>> +		goto err;
>> +	}
>> +	dev_set_drvdata(&event_dev->dev, event_dev);
>> +
>> +	/* Add devfreq event device to devfreq_event_list */
>> +	INIT_LIST_HEAD(&event_dev->node);
>> +	list_add(&event_dev->node, &devfreq_event_list);
>> +
>> +	mutex_unlock(&devfreq_event_list_lock);
>> +
>> +	return event_dev;
>> +err:
> 
> Missing 'mutex_unlock(&devfreq_event_list_lock)' here.

OK. I'll fix it.

> 
> 
>> +	kfree(event_dev);
>> +	return ERR_PTR(ret);
>> +}
>> +EXPORT_SYMBOL(devfreq_add_event_device);
>> +
>> +struct devfreq_event_dev *devfreq_get_event_dev(const char *event_dev_name)
>> +{
>> +	struct devfreq_event_dev *event_dev;
>> +
>> +	mutex_lock(&devfreq_event_list_lock);
>> +	list_for_each_entry(event_dev, &devfreq_event_list, node) {
>> +		if (!strcmp(event_dev->desc->name, event_dev_name))
>> +			goto out;
>> +	}
>> +	event_dev = NULL;
>> +out:
>> +	mutex_unlock(&devfreq_event_list_lock);
>> +
>> +	return event_dev;
>> +}
>> +EXPORT_SYMBOL_GPL(devfreq_get_event_dev);
>> +
>> +struct devfreq_event_dev *devfreq_get_event_dev_by_phandle(struct device *dev,
>> +						      int index)
>> +{
>> +	struct device_node *node;
>> +	struct devfreq_event_dev *event_dev;
>> +
>> +	if (!dev->of_node) {
>> +		dev_err(dev, "device does not have a device node entry\n");
>> +		return ERR_PTR(-EINVAL);
>> +	}
>> +
>> +	node = of_parse_phandle(dev->of_node, "devfreq-events", index);
>> +	if (!node) {
>> +		dev_err(dev, "failed to get phandle in %s node\n",
>> +			dev->of_node->full_name);
>> +		return ERR_PTR(-ENODEV);
>> +	}
>> +
>> +	event_dev = devfreq_get_event_dev(node->name);
>> +	if (!event_dev) {
>> +		dev_err(dev, "unable to get devfreq-event device : %s\n",
>> +			node->name);
> 
> of_node_put() for node obtained with of_parse_phandle().

OK. I'll add it.

> 
>> +		return ERR_PTR(-ENODEV);
>> +	}
>> +
>> +	return event_dev;
>> +}
>> +EXPORT_SYMBOL_GPL(devfreq_get_event_dev_by_phandle);
>> +
>> +int devfreq_put_event_dev(struct devfreq_event_dev *event_dev)
>> +{
> 
> of_node_put() here to decrement refcnt from of_parse_phandle()?

It is my mistake. I think that devfreq_put_event_dev is not necesssary.
I'll remove it.

> 
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(devfreq_put_event_dev);
>> +
>> +int devfreq_enable_event_dev(struct devfreq_event_dev *event_dev)
>> +{
>> +	int ret = 0;
>> +
>> +	if (!event_dev || !event_dev->desc)
>> +		return -EINVAL;
>> +
>> +	mutex_lock(&event_dev->lock);
>> +	if (event_dev->desc->ops && event_dev->desc->ops->enable) {
>> +		ret = event_dev->desc->ops->enable(event_dev);
>> +		if (ret < 0)
>> +			goto err;
>> +	}
>> +	event_dev->enable_count++;
>> +err:
>> +	mutex_unlock(&event_dev->lock);
>> +
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(devfreq_enable_event_dev);
>> +
>> +int devfreq_disable_event_dev(struct devfreq_event_dev *event_dev)
>> +{
>> +	int ret = 0;
>> +
>> +	if (!event_dev || !event_dev->desc)
>> +		return -EINVAL;
>> +
>> +	mutex_lock(&event_dev->lock);
>> +	if (event_dev->enable_count > 0) {
>> +		event_dev->enable_count--;
>> +	} else {
>> +		dev_warn(&event_dev->dev, "unbalanced enable_count\n");
>> +		ret = -EINVAL;
>> +		goto err;
>> +	}
>> +
>> +	if (event_dev->desc->ops && event_dev->desc->ops->disable) {
>> +		ret = event_dev->desc->ops->disable(event_dev);
>> +		if (ret < 0) {
>> +			event_dev->enable_count++;
>> +			goto err;
>> +		}
>> +	}
>> +err:
>> +	mutex_unlock(&event_dev->lock);
>> +
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(devfreq_disable_event_dev);
>> +
>> +bool devfreq_is_enabled_event_dev(struct devfreq_event_dev *event_dev)
>> +{
>> +	bool enabled = false;
>> +
>> +	if (!event_dev || !event_dev->desc)
>> +		return enabled;
>> +
>> +	if (!(event_dev->enable_count > 0))
>> +		return enabled;
>> +
>> +	if (event_dev->desc->ops && event_dev->desc->ops->is_enabled)
>> +		enabled = event_dev->desc->ops->is_enabled(event_dev);
>> +
>> +	return enabled;
>> +}
>> +EXPORT_SYMBOL_GPL(devfreq_is_enabled_event_dev);
>> +
>> +int devfreq_set_event_event_dev(struct devfreq_event_dev *event_dev)
> 
> The convention of names you use is not obvious to me. I would expect
> rather devfreq_event_dev_XXX (where XXX is "set_event", "is_enabled"
> etc).
> The one above is good example what is the issue with current convention:
> devfreq_set_event_event_dev
>             ^     ^
> This double "event" looks weird.

You're right. I'll have to fix the function name according to your comment.

> 
>> +{
>> +	if (!event_dev || !event_dev->desc)
>> +		return -EINVAL;
>> +
>> +	if (!devfreq_is_enabled_event_dev(event_dev))
>> +		return -EPERM;
>> +
>> +	if (event_dev->desc->ops && event_dev->desc->ops->set_event)
>> +		return event_dev->desc->ops->set_event(event_dev);
> 
> No mutexes here? What exactly is protected by mutex?

I used the mutex when devfreq-event class read/write the data of devfreq_event_dev structure.

> 
>> +
>> +	return -EINVAL;
>> +}
>> +EXPORT_SYMBOL_GPL(devfreq_set_event_event_dev);
>> +
>> +int devfreq_get_event_event_dev(struct devfreq_event_dev *event_dev,
>> +				int *total_event)
>> +{
>> +	if (!event_dev || !event_dev->desc)
>> +		return -EINVAL;
>> +
>> +	if (!devfreq_is_enabled_event_dev(event_dev))
>> +		return -EPERM;
>> +
>> +	if (event_dev->desc->ops && event_dev->desc->ops->get_event)
>> +		return event_dev->desc->ops->get_event(event_dev, total_event);
>> +
>> +	return -EINVAL;
>> +}
>> +EXPORT_SYMBOL_GPL(devfreq_get_event_event_dev);
>> +
>> +int devfreq_reset_event_dev(struct devfreq_event_dev *event_dev)
>> +{
>> +	if (!event_dev || !event_dev->desc)
>> +		return -EINVAL;
>> +
>> +	if (!devfreq_is_enabled_event_dev(event_dev))
>> +		return -EPERM;
>> +
>> +	if (event_dev->desc->ops && event_dev->desc->ops->reset)
>> +		return event_dev->desc->ops->reset(event_dev);
> 
> Same here, no mutex?

I replied it on previous your question.

Best Regards,
Chanwoo Choi

Re: [RFC PATCHv2 4/7] devfreq: event: Add exynos-ppmu devfreq event driver

[Chanwoo Choi]

Hi Krzysztof,

On 12/10/2014 06:59 PM, Krzysztof Kozlowski wrote:
> On wto, 2014-12-09 at 23:13 +0900, Chanwoo Choi wrote:
>> This patch add exynos-ppmu devfreq event driver to provider raw data about
>> the utilization of each IP in Exynos SoC series.
>>
>> Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
>> Cc: Kyungmin Park <kyungmin.park@samsung.com>
>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>> ---
>>  drivers/devfreq/Kconfig             |   9 +
>>  drivers/devfreq/event/Makefile      |   1 +
>>  drivers/devfreq/event/exynos-ppmu.c | 409 ++++++++++++++++++++++++++++++++++++
>>  3 files changed, 419 insertions(+)
>>  create mode 100644 drivers/devfreq/event/exynos-ppmu.c
> 
> I would rather see this as an incremental change for existing
> exynos_ppmu.c file. However I do not insists on that.
> 
>>
>> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
>> index 4d15b62..d4559f7 100644
>> --- a/drivers/devfreq/Kconfig
>> +++ b/drivers/devfreq/Kconfig
>> @@ -89,4 +89,13 @@ config ARM_EXYNOS5_BUS_DEVFREQ
>>  
>>  comment "DEVFREQ Event Drivers"
>>  
>> +config DEVFREQ_EVENT_EXYNOS_PPMU
>> +	bool "EXYNOS PPMU (Performance Profiling Monitoring Unit) DEVFREQ event Driver"
>> +	depends on ARCH_EXYNOS
>> +	select PM_OPP
>> +	help
>> +	 This add the DEVFREQ event driver for Exynos SoC. It provides PPMU
>> +	 (Performance Profiling Monitoring Unit) counters to estimate the
>> +	 utilization of each module.
>> +
>>  endif # PM_DEVFREQ
>> diff --git a/drivers/devfreq/event/Makefile b/drivers/devfreq/event/Makefile
>> index dc56005..be146ea 100644
>> --- a/drivers/devfreq/event/Makefile
>> +++ b/drivers/devfreq/event/Makefile
>> @@ -1 +1,2 @@
>>  # Exynos DEVFREQ Event Drivers
>> +obj-$(CONFIG_DEVFREQ_EVENT_EXYNOS_PPMU) += exynos-ppmu.o
>> diff --git a/drivers/devfreq/event/exynos-ppmu.c b/drivers/devfreq/event/exynos-ppmu.c
>> new file mode 100644
>> index 0000000..2706f23
>> --- /dev/null
>> +++ b/drivers/devfreq/event/exynos-ppmu.c
>> @@ -0,0 +1,409 @@
>> +/*
>> + * exynos_ppmu.c - EXYNOS PPMU (Performance Profiling Monitoring Units) support
>> + *
>> + * Copyright (c) 2014 Samsung Electronics Co., Ltd.
>> + * Author : Chanwoo Choi <cw00.choi@samsung.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.
>> + *
>> + * This driver is based on drivers/devfreq/exynos/exynos_ppmu.c
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/io.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/mutex.h>
>> +#include <linux/of_address.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/suspend.h>
>> +#include <linux/devfreq.h>
>> +
>> +#define PPMU_ENABLE             BIT(0)
>> +#define PPMU_DISABLE            0x0
>> +#define PPMU_CYCLE_RESET        BIT(1)
>> +#define PPMU_COUNTER_RESET      BIT(2)
>> +
>> +#define PPMU_ENABLE_COUNT0      BIT(0)
>> +#define PPMU_ENABLE_COUNT1      BIT(1)
>> +#define PPMU_ENABLE_COUNT2      BIT(2)
>> +#define PPMU_ENABLE_COUNT3      BIT(3)
>> +#define PPMU_ENABLE_CYCLE       BIT(31)
>> +
>> +#define PPMU_CNTENS		0x10
>> +#define PPMU_FLAG		0x50
>> +#define PPMU_CCNT_OVERFLOW	BIT(31)
>> +#define PPMU_CCNT		0x100
>> +
>> +#define PPMU_PMCNT0		0x110
>> +#define PPMU_PMCNT_OFFSET	0x10
>> +#define PMCNT_OFFSET(x)		(PPMU_PMCNT0 + (PPMU_PMCNT_OFFSET * x))
>> +
>> +#define PPMU_BEVT0SEL		0x1000
>> +#define PPMU_BEVTSEL_OFFSET	0x100
>> +#define PPMU_BEVTSEL(x)		(PPMU_BEVT0SEL + (x * PPMU_BEVTSEL_OFFSET))
>> +
>> +#define RD_DATA_COUNT		0x5
>> +#define WR_DATA_COUNT		0x6
>> +#define RDWR_DATA_COUNT		0x7
>> +
>> +enum ppmu_counter {
>> +	PPMU_PMNCNT0,
>> +	PPMU_PMNCNT1,
>> +	PPMU_PMNCNT2,
>> +	PPMU_PMNCNT3,
>> +	PPMU_PMNCNT_MAX,
>> +};
>> +
>> +/* Platform data */
>> +struct exynos_ppmu_data {
>> +	struct devfreq *devfreq;
> 
> Looks like not used here.

Right. I'll remove it.

> 
>> +	struct devfreq_event_dev **event_dev;
>> +	struct devfreq_event_desc *desc;
>> +	unsigned int num_events;
>> +
>> +	struct device *dev;
>> +	struct clk *clk_ppmu;
>> +	struct mutex lock;
>> +
>> +	struct __exynos_ppmu {
>> +		void __iomem *hw_base;
>> +		unsigned int ccnt;
>> +		unsigned int event[PPMU_PMNCNT_MAX];
>> +		unsigned int count[PPMU_PMNCNT_MAX];
>> +		unsigned long long ns;
>> +		ktime_t reset_time;
>> +		bool ccnt_overflow;
>> +		bool count_overflow[PPMU_PMNCNT_MAX];
>> +	} ppmu;
>> +};
>> +
>> +struct __exynos_ppmu_events {
>> +	char *name;
>> +	int id;
>> +} ppmu_events[] = {
>> +	{ "ppmu-dmc0-pmcnt0", PPMU_PMNCNT0 },
>> +	{ "ppmu-dmc0-pmcnt1", PPMU_PMNCNT1 },
>> +	{ "ppmu-dmc0-pmcnt2", PPMU_PMNCNT2 },
>> +	{ "ppmu-dmc0-pmcnt3", PPMU_PMNCNT3 },
>> +
>> +	{ "ppmu-dmc1-pmcnt0", PPMU_PMNCNT0 },
>> +	{ "ppmu-dmc1-pmcnt1", PPMU_PMNCNT1 },
>> +	{ "ppmu-dmc1-pmcnt2", PPMU_PMNCNT2 },
>> +	{ "ppmu-dmc1-pmcnt3", PPMU_PMNCNT3 },
>> +
>> +	{ "ppmu-cpu-pmcnt0", PPMU_PMNCNT0 },
>> +	{ "ppmu-cpu-pmcnt1", PPMU_PMNCNT1 },
>> +	{ "ppmu-cpu-pmcnt2", PPMU_PMNCNT2 },
>> +	{ "ppmu-cpu-pmcnt3", PPMU_PMNCNT3 },
>> +
>> +	{ "ppmu-rightbus-pmcnt0", PPMU_PMNCNT0 },
>> +	{ "ppmu-rightbus-pmcnt1", PPMU_PMNCNT1 },
>> +	{ "ppmu-rightbus-pmcnt2", PPMU_PMNCNT2 },
>> +	{ "ppmu-rightbus-pmcnt3", PPMU_PMNCNT3 },
>> +
>> +	{ "ppmu-leftbus-pmcnt0", PPMU_PMNCNT0 },
>> +	{ "ppmu-leftbus-pmcnt1", PPMU_PMNCNT1 },
>> +	{ "ppmu-leftbus-pmcnt2", PPMU_PMNCNT2 },
>> +	{ "ppmu-leftbus-pmcnt3", PPMU_PMNCNT3 },
>> +
>> +	{ "ppmu-camif-pmcnt0", PPMU_PMNCNT0 },
>> +	{ "ppmu-camif-pmcnt1", PPMU_PMNCNT1 },
>> +	{ "ppmu-camif-pmcnt2", PPMU_PMNCNT2 },
>> +	{ "ppmu-camif-pmcnt3", PPMU_PMNCNT3 },
>> +
>> +	{ "ppmu-lcd0-pmcnt0", PPMU_PMNCNT0 },
>> +	{ "ppmu-lcd0-pmcnt1", PPMU_PMNCNT1 },
>> +	{ "ppmu-lcd0-pmcnt2", PPMU_PMNCNT2 },
>> +	{ "ppmu-lcd0-pmcnt3", PPMU_PMNCNT3 },
>> +
>> +	{ "ppmu-g3d-pmcnt0", PPMU_PMNCNT0 },
>> +	{ "ppmu-g3d-pmcnt1", PPMU_PMNCNT1 },
>> +	{ "ppmu-g3d-pmcnt2", PPMU_PMNCNT2 },
>> +	{ "ppmu-g3d-pmcnt3", PPMU_PMNCNT3 },
>> +
>> +	{ "ppmu-mfc-pmcnt0", PPMU_PMNCNT0 },
>> +	{ "ppmu-mfc-pmcnt1", PPMU_PMNCNT1 },
>> +	{ "ppmu-mfc-pmcnt2", PPMU_PMNCNT2 },
>> +	{ "ppmu-mfc-pmcnt3", PPMU_PMNCNT3 },
>> +
>> +	{ "ppmu-fsys-pmcnt0", PPMU_PMNCNT0 },
>> +	{ "ppmu-fsys-pmcnt1", PPMU_PMNCNT1 },
>> +	{ "ppmu-fsys-pmcnt2", PPMU_PMNCNT2 },
>> +	{ "ppmu-fsys-pmcnt3", PPMU_PMNCNT3 },
>> +	{ /* sentinel */ },
>> +};
>> +
>> +static int exynos_ppmu_enable(struct devfreq_event_dev *event_dev)
>> +{
>> +	struct exynos_ppmu_data *exynos_ppmu = event_dev_get_drvdata(event_dev);
>> +
>> +	__raw_writel(PPMU_ENABLE, exynos_ppmu->ppmu.hw_base);
>> +
>> +	return 0;
>> +}
>> +
>> +static int exynos_ppmu_disable(struct devfreq_event_dev *event_dev)
>> +{
>> +	struct exynos_ppmu_data *exynos_ppmu = event_dev_get_drvdata(event_dev);
>> +
>> +	__raw_writel(PPMU_DISABLE, exynos_ppmu->ppmu.hw_base);
>> +
>> +	return 0;
>> +}
>> +
>> +static bool exynos_ppmu_is_enabled(struct devfreq_event_dev *event_dev)
>> +{
>> +	struct exynos_ppmu_data *exynos_ppmu = event_dev_get_drvdata(event_dev);
>> +	int val;
>> +
>> +	val = __raw_readl(exynos_ppmu->ppmu.hw_base);
>> +	if (val & PPMU_ENABLE)
>> +		return true;
>> +
>> +	return false;
>> +}
>> +
>> +static int exynos_ppmu_find_ppmu_id(struct devfreq_event_dev *event_dev)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < ARRAY_SIZE(ppmu_events); i++)
>> +		if (!strcmp(event_dev->desc->name, ppmu_events[i].name))
>> +			return ppmu_events[i].id;
>> +
>> +	return -EINVAL;
>> +}
>> +
>> +static int exynos_ppmu_set_event(struct devfreq_event_dev *event_dev,
>> +				enum devfreq_event_type event_type)
>> +{
>> +	struct exynos_ppmu_data *exynos_ppmu = event_dev_get_drvdata(event_dev);
>> +	void __iomem *ppmu_base = exynos_ppmu->ppmu.hw_base;
>> +	int id = exynos_ppmu_find_ppmu_id(event_dev);
>> +
>> +	__raw_writel(RDWR_DATA_COUNT, ppmu_base + PPMU_BEVTSEL(id));
>> +
>> +	return 0;
>> +}
>> +
>> +static int exynos_ppmu_get_event(struct devfreq_event_dev *event_dev,
>> +				enum devfreq_event_type event_type,
>> +				int *total_event)
>> +{
>> +	struct exynos_ppmu_data *exynos_ppmu = event_dev_get_drvdata(event_dev);
>> +	void __iomem *ppmu_base = exynos_ppmu->ppmu.hw_base;
>> +	int id = exynos_ppmu_find_ppmu_id(event_dev);
>> +	int cnt, total_cnt;
>> +
>> +	total_cnt = __raw_readl(ppmu_base + PPMU_CCNT);
>> +
>> +	if (id == PPMU_PMNCNT3)
>> +		cnt = ((__raw_readl(ppmu_base + PMCNT_OFFSET(id)) << 8) |
>> +			  __raw_readl(ppmu_base + PMCNT_OFFSET(id + 1)));
>> +	else
>> +		cnt = __raw_readl(ppmu_base + PMCNT_OFFSET(id));
>> +
>> +	*total_event = total_cnt;
>> +
>> +	return cnt;
>> +}
>> +
>> +static int exynos_ppmu_reset(struct devfreq_event_dev *event_dev)
>> +{
>> +	struct exynos_ppmu_data *exynos_ppmu = event_dev_get_drvdata(event_dev);
>> +	void __iomem *ppmu_base = exynos_ppmu->ppmu.hw_base;
>> +
>> +	__raw_writel(PPMU_CYCLE_RESET | PPMU_COUNTER_RESET, ppmu_base);
>> +	__raw_writel(PPMU_ENABLE_CYCLE  |
>> +		     PPMU_ENABLE_COUNT0 |
>> +		     PPMU_ENABLE_COUNT1 |
>> +		     PPMU_ENABLE_COUNT2 |
>> +		     PPMU_ENABLE_COUNT3,
>> +		     ppmu_base + PPMU_CNTENS);
>> +	__raw_writel(PPMU_ENABLE, exynos_ppmu->ppmu.hw_base);
>> +
>> +	return 0;
>> +}
>> +
>> +static struct devfreq_event_ops exynos_ppmu_ops = {
>> +	.enable		= exynos_ppmu_enable,
>> +	.disable	= exynos_ppmu_disable,
>> +	.is_enabled	= exynos_ppmu_is_enabled,
>> +	.set_event	= exynos_ppmu_set_event,
>> +	.get_event	= exynos_ppmu_get_event,
>> +	.reset		= exynos_ppmu_reset,
>> +};
>> +
>> +static int of_get_devfreq_events(struct device_node *np,
>> +				struct exynos_ppmu_data *exynos_ppmu)
>> +{
>> +	struct devfreq_event_desc *desc;
>> +	struct device *dev = exynos_ppmu->dev;
>> +	struct device_node *events_np, *node;
>> +	int i, j, count;
>> +
>> +	events_np = of_find_node_by_name(np, "events");
> 
> of_get_child_by_name()

OK. I'll use of_get_chilc_by_name() function to get phandle of devfreq-event device in dts.

> 
>> +	if (!events_np) {
>> +		dev_err(dev, "Failed to find ppmus sub-node\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	count = of_get_child_count(events_np);
>> +	desc = devm_kzalloc(dev, sizeof(struct devfreq_event_desc) * count,
>> +			      GFP_KERNEL);
>> +	if (!desc)
>> +		return -ENOMEM;
>> +	exynos_ppmu->num_events = count;
>> +
>> +	j = 0;
>> +	for_each_child_of_node(events_np, node) {
>> +		for (i = 0; i < ARRAY_SIZE(ppmu_events); i++) {
>> +			if (!of_node_cmp(node->name, ppmu_events[i].name))
>> +				break;
>> +		}
>> +
>> +		if (i == ARRAY_SIZE(ppmu_events)) {
>> +			dev_warn(dev,
>> +				"don't know how to configure events : %s\n",
>> +				node->name);
>> +			continue;
>> +		}
>> +		desc[j].ops = &exynos_ppmu_ops;
>> +		desc[j].driver_data = exynos_ppmu;
>> +		desc[j].event_type |= DEVFREQ_EVENT_TYPE_RAW_DATA;
>> +		of_property_read_string(node, "event-name", &desc[j].name);
>> +		j++;
>> +	}
>> +	exynos_ppmu->desc = desc;
> 
> of_node_put() for 'events_np' and 'node' in loop.

OK, I'll add it.

> 
>> +
>> +	return 0;
>> +}
>> +
>> +static int exynos_ppmu_parse_dt(struct exynos_ppmu_data *exynos_ppmu)
>> +{
>> +	struct device *dev = exynos_ppmu->dev;
>> +	struct device_node *np = dev->of_node;
>> +	int ret = 0;
>> +
>> +	if (!np) {
>> +		dev_err(dev, "Failed to find devicetree node\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	/* Maps the memory mapped IO to control PPMU register */
>> +	exynos_ppmu->ppmu.hw_base = of_iomap(np, 0);
>> +	if (IS_ERR_OR_NULL(exynos_ppmu->ppmu.hw_base)) {
>> +		dev_err(dev, "Failed to map memory region\n");
>> +		ret = -EINVAL;
>> +		goto err_iomap;
>> +	}
>> +
>> +	/* FIXME: Get the clock of ppmu and enable this clock */
>> +	exynos_ppmu->clk_ppmu = devm_clk_get(dev, "ppmu");
>> +	if (IS_ERR(exynos_ppmu->clk_ppmu))
>> +		dev_warn(dev, "Failed to get PPMU clock\n");
>> +
>> +	ret = of_get_devfreq_events(np, exynos_ppmu);
>> +	if (ret < 0) {
>> +		dev_err(dev, "Failed to parse exynos ppmu dt node\n");
>> +		goto err_clock;
>> +	}
>> +
>> +	return 0;
>> +
>> +err_clock:
>> +	clk_disable_unprepare(exynos_ppmu->clk_ppmu);
> 
> Not needed. Clock is not enabled here.

OK. I'll fix it.

> 
>> +err_iomap:
>> +	iounmap(exynos_ppmu->ppmu.hw_base);
>> +
>> +	return ret;
>> +}
>> +
>> +static int exynos_ppmu_probe(struct platform_device *pdev)
>> +{
>> +	struct exynos_ppmu_data *exynos_ppmu;
>> +	struct devfreq_event_dev **event_dev;
>> +	struct devfreq_event_desc *desc;
>> +	int i, ret = 0, size;
>> +
>> +	/* Allocate the memory of exynos_ppmu_data and initialize it */
>> +	exynos_ppmu = devm_kzalloc(&pdev->dev, sizeof(struct exynos_ppmu_data),
>> +				   GFP_KERNEL);
>> +	if (!exynos_ppmu)
>> +		return -ENOMEM;
>> +
>> +	mutex_init(&exynos_ppmu->lock);
>> +	exynos_ppmu->dev = &pdev->dev;
>> +
>> +	/* Parse dt data to get resource */
>> +	ret = exynos_ppmu_parse_dt(exynos_ppmu);
>> +	if (ret < 0) {
>> +		dev_err(&pdev->dev, "Failed to parse DT node for resource\n");
>> +		return ret;
>> +	}
>> +	desc = exynos_ppmu->desc;
>> +
>> +	size = sizeof(struct devfreq_event_dev *) * exynos_ppmu->num_events;
>> +	exynos_ppmu->event_dev = devm_kzalloc(&pdev->dev, size, GFP_KERNEL);
>> +	if (!exynos_ppmu->event_dev) {
>> +		dev_err(&pdev->dev,
>> +			"Failed to allocate memory devfreq_event_dev list\n");
>> +		return -ENOMEM;
>> +	}
>> +	event_dev = exynos_ppmu->event_dev;
>> +	platform_set_drvdata(pdev, exynos_ppmu);
> 
> Missing clk_prepare_enable().

OK, I'll add it.

> 
>> +
>> +	for (i = 0; i < exynos_ppmu->num_events; i++) {
>> +		event_dev[i] = devfreq_add_event_device(&pdev->dev, &desc[i]);
>> +		if (IS_ERR(event_dev)) {
>> +			ret = PTR_ERR(event_dev);
>> +			dev_err(&pdev->dev, "Failed to add devfreq evt dev\n");
>> +			goto err;
>> +		}
>> +	}
>> +
>> +	return 0;
>> +err:
>> +	clk_disable_unprepare(exynos_ppmu->clk_ppmu);
>> +	iounmap(exynos_ppmu->ppmu.hw_base);
>> +
>> +	return ret;
>> +}
>> +
>> +static int exynos_ppmu_remove(struct platform_device *pdev)
>> +{
>> +	struct exynos_ppmu_data *exynos_ppmu = platform_get_drvdata(pdev);
>> +
>> +	clk_disable_unprepare(exynos_ppmu->clk_ppmu);
>> +	iounmap(exynos_ppmu->ppmu.hw_base);
>> +
>> +	/* Remove devfreq instance */
>> +	devfreq_remove_device(exynos_ppmu->devfreq);
> 
> Shouldn't this be devfreq_remove_event_dev()?

Your right. It is my mistake. I'll fix it.

Thanks for your review.

Best Regards,
Chanwoo Choi

Re: [RFC PATCHv2 1/7] devfreq: event: Add new devfreq_event class to provide basic data for devfreq governor

[Chanwoo Choi]

Hi Krzysztof,

I replied again this mail because I'll use the mutex for set_event()/get_event()
according to your comment. But, of_parse_phandle() seems that this function
don't need the of_node_put() function.


On 12/11/2014 11:13 AM, Chanwoo Choi wrote:
> Hi Krzysztof,
> 
> First of all, thanks for your review.
> 
> On 12/10/2014 06:37 PM, Krzysztof Kozlowski wrote:
>> On wto, 2014-12-09 at 23:13 +0900, Chanwoo Choi wrote:
>>> This patch add new devfreq_event class for devfreq_event device which provide
>>> raw data (e.g., memory bus utilization/GPU utilization). This raw data from
>>> devfreq_event data would be used for the governor of devfreq subsystem.
>>> - devfreq_event device : Provide raw data for governor of existing devfreq device
>>> - devfreq device       : Monitor device state and change frequency/voltage of device
>>>                          using the raw data from devfreq_event device
>>>
>>> The devfreq subsystem support generic DVFS(Dynamic Voltage/Frequency Scaling)
>>> for Non-CPU Devices. The devfreq device would dertermine current device state
>>> using various governor (e.g., ondemand, performance, powersave). After completed
>>> determination of system state, devfreq device would change the frequency/voltage
>>> of devfreq device according to the result of governor.
>>>
>>> But, devfreq governor must need basic data which indicates current device state.
>>> Existing devfreq subsystem only consider devfreq device which check current system
>>> state and determine proper system state using basic data. There is no subsystem
>>> for device providing basic data to devfreq device.
>>>
>>> The devfreq subsystem must need devfreq_event device(data-provider device) for
>>> existing devfreq device. So, this patch add new devfreq_event class for
>>> devfreq_event device which read various basic data(e.g, memory bus utilization,
>>> GPU utilization) and provide measured data to existing devfreq device through
>>> standard APIs of devfreq_event class.
>>>
>>> The following description explains the feature of two kind of devfreq class:
>>> - devfreq class (existing)
>>>  : devfreq consumer device use raw data from devfreq_event device for
>>>    determining proper current system state and change voltage/frequency
>>>    dynamically using various governors.
>>>
>>> - devfreq_event class (new)
>>>  : Provide measured raw data to devfreq device for governor
>>>
>>> Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
>>> Cc: Kyungmin Park <kyungmin.park@samsung.com>
>>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>>> ---
>>>  drivers/devfreq/Kconfig         |   2 +
>>>  drivers/devfreq/Makefile        |   5 +-
>>>  drivers/devfreq/devfreq-event.c | 302 ++++++++++++++++++++++++++++++++++++++++
>>>  drivers/devfreq/event/Makefile  |   1 +
>>>  include/linux/devfreq.h         | 141 +++++++++++++++++++
>>>  5 files changed, 450 insertions(+), 1 deletion(-)
>>>  create mode 100644 drivers/devfreq/devfreq-event.c
>>>  create mode 100644 drivers/devfreq/event/Makefile
>>>
>>> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
>>> index faf4e70..4d15b62 100644
>>> --- a/drivers/devfreq/Kconfig
>>> +++ b/drivers/devfreq/Kconfig
>>> @@ -87,4 +87,6 @@ config ARM_EXYNOS5_BUS_DEVFREQ
>>>  	  It reads PPMU counters of memory controllers and adjusts the
>>>  	  operating frequencies and voltages with OPP support.
>>>  
>>> +comment "DEVFREQ Event Drivers"
>>> +
>>>  endif # PM_DEVFREQ
>>> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
>>> index 16138c9..a1ffabe 100644
>>> --- a/drivers/devfreq/Makefile
>>> +++ b/drivers/devfreq/Makefile
>>> @@ -1,4 +1,4 @@
>>> -obj-$(CONFIG_PM_DEVFREQ)	+= devfreq.o
>>> +obj-$(CONFIG_PM_DEVFREQ)		+= devfreq.o devfreq-event.o
>>>  obj-$(CONFIG_DEVFREQ_GOV_SIMPLE_ONDEMAND)	+= governor_simpleondemand.o
>>>  obj-$(CONFIG_DEVFREQ_GOV_PERFORMANCE)	+= governor_performance.o
>>>  obj-$(CONFIG_DEVFREQ_GOV_POWERSAVE)	+= governor_powersave.o
>>> @@ -7,3 +7,6 @@ obj-$(CONFIG_DEVFREQ_GOV_USERSPACE)	+= governor_userspace.o
>>>  # DEVFREQ Drivers
>>>  obj-$(CONFIG_ARM_EXYNOS4_BUS_DEVFREQ)	+= exynos/
>>>  obj-$(CONFIG_ARM_EXYNOS5_BUS_DEVFREQ)	+= exynos/
>>> +
>>> +# DEVFREQ Event Drivers
>>> +obj-$(CONFIG_PM_DEVFREQ)		+= event/
>>> diff --git a/drivers/devfreq/devfreq-event.c b/drivers/devfreq/devfreq-event.c
>>> new file mode 100644
>>> index 0000000..b47329f
>>> --- /dev/null
>>> +++ b/drivers/devfreq/devfreq-event.c
>>> @@ -0,0 +1,302 @@
>>> +/*
>>> + * devfreq-event: Generic DEVFREQ Event class driver
>>> + *
>>> + * Copyright (C) 2014 Samsung Electronics
>>> + * Chanwoo Choi <cw00.choi@samsung.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.
>>> + *
>>> + * This driver is based on drivers/devfreq/devfreq.c
>>> + */
>>> +
>>> +#include <linux/kernel.h>
>>> +#include <linux/sched.h>
>>> +#include <linux/errno.h>
>>> +#include <linux/err.h>
>>> +#include <linux/init.h>
>>> +#include <linux/module.h>
>>> +#include <linux/slab.h>
>>> +#include <linux/stat.h>
>>> +#include <linux/pm_opp.h>
>>> +#include <linux/devfreq.h>
>>> +#include <linux/workqueue.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/list.h>
>>> +#include <linux/printk.h>
>>> +#include <linux/hrtimer.h>
>>> +#include <linux/of.h>
>>> +#include "governor.h"
>>> +
>>> +static struct class *devfreq_event_class;
>>> +
>>> +/* The list of all devfreq event list */
>>> +static LIST_HEAD(devfreq_event_list);
>>> +static DEFINE_MUTEX(devfreq_event_list_lock);
>>> +
>>> +#define to_devfreq_event(DEV) container_of(DEV, struct devfreq_event_dev, dev)
>>> +
>>> +struct devfreq_event_dev *devfreq_add_event_device(struct device *dev,
>>> +					struct devfreq_event_desc *desc)
>>> +{
>>> +	struct devfreq_event_dev *event_dev;
>>> +	static atomic_t event_no = ATOMIC_INIT(0);
>>> +	int ret;
>>> +
>>> +	if (!dev || !desc)
>>> +		return ERR_PTR(-EINVAL);
>>> +
>>> +	if (!desc->name || !desc->ops)
>>> +		return ERR_PTR(-EINVAL);
>>> +
>>> +	event_dev = kzalloc(sizeof(struct devfreq_event_dev), GFP_KERNEL);
>>> +	if (!event_dev)
>>> +		return ERR_PTR(-ENOMEM);
>>
>> Is this memory freed anywhere when driver is removed? I couldn't find
>> it. I couldn't also find function like devfreq_remove_event_device()
>> which would be reverting all the work done when adding.
> 
> You're right. I'll use devm_kzalloc instead of kzalloc.
> 
>>
>>> +
>>> +	mutex_lock(&devfreq_event_list_lock);
>>> +
>>> +	mutex_init(&event_dev->lock);
>>> +	event_dev->desc = desc;
>>> +	event_dev->dev.parent = dev;
>>> +	event_dev->dev.class = devfreq_event_class;
>>> +
>>> +	dev_set_name(&event_dev->dev, "event.%d",
>>> +		     atomic_inc_return(&event_no) - 1);
>>> +	ret = device_register(&event_dev->dev);
>>> +	if (ret != 0) {
>>> +		put_device(&event_dev->dev);
>>> +		goto err;
>>> +	}
>>> +	dev_set_drvdata(&event_dev->dev, event_dev);
>>> +
>>> +	/* Add devfreq event device to devfreq_event_list */
>>> +	INIT_LIST_HEAD(&event_dev->node);
>>> +	list_add(&event_dev->node, &devfreq_event_list);
>>> +
>>> +	mutex_unlock(&devfreq_event_list_lock);
>>> +
>>> +	return event_dev;
>>> +err:
>>
>> Missing 'mutex_unlock(&devfreq_event_list_lock)' here.
> 
> OK. I'll fix it.
> 
>>
>>
>>> +	kfree(event_dev);
>>> +	return ERR_PTR(ret);
>>> +}
>>> +EXPORT_SYMBOL(devfreq_add_event_device);
>>> +
>>> +struct devfreq_event_dev *devfreq_get_event_dev(const char *event_dev_name)
>>> +{
>>> +	struct devfreq_event_dev *event_dev;
>>> +
>>> +	mutex_lock(&devfreq_event_list_lock);
>>> +	list_for_each_entry(event_dev, &devfreq_event_list, node) {
>>> +		if (!strcmp(event_dev->desc->name, event_dev_name))
>>> +			goto out;
>>> +	}
>>> +	event_dev = NULL;
>>> +out:
>>> +	mutex_unlock(&devfreq_event_list_lock);
>>> +
>>> +	return event_dev;
>>> +}
>>> +EXPORT_SYMBOL_GPL(devfreq_get_event_dev);
>>> +
>>> +struct devfreq_event_dev *devfreq_get_event_dev_by_phandle(struct device *dev,
>>> +						      int index)
>>> +{
>>> +	struct device_node *node;
>>> +	struct devfreq_event_dev *event_dev;
>>> +
>>> +	if (!dev->of_node) {
>>> +		dev_err(dev, "device does not have a device node entry\n");
>>> +		return ERR_PTR(-EINVAL);
>>> +	}
>>> +
>>> +	node = of_parse_phandle(dev->of_node, "devfreq-events", index);
>>> +	if (!node) {
>>> +		dev_err(dev, "failed to get phandle in %s node\n",
>>> +			dev->of_node->full_name);
>>> +		return ERR_PTR(-ENODEV);
>>> +	}
>>> +
>>> +	event_dev = devfreq_get_event_dev(node->name);
>>> +	if (!event_dev) {
>>> +		dev_err(dev, "unable to get devfreq-event device : %s\n",
>>> +			node->name);
>>
>> of_node_put() for node obtained with of_parse_phandle().
> 
> OK. I'll add it.

of_parse_phandle() seems that it don't need of_node_put().

> 
>>
>>> +		return ERR_PTR(-ENODEV);
>>> +	}
>>> +
>>> +	return event_dev;
>>> +}
>>> +EXPORT_SYMBOL_GPL(devfreq_get_event_dev_by_phandle);
>>> +
>>> +int devfreq_put_event_dev(struct devfreq_event_dev *event_dev)
>>> +{
>>
>> of_node_put() here to decrement refcnt from of_parse_phandle()?
> 
> It is my mistake. I think that devfreq_put_event_dev is not necesssary.
> I'll remove it.
> 
>>
>>> +	return 0;
>>> +}
>>> +EXPORT_SYMBOL_GPL(devfreq_put_event_dev);
>>> +
>>> +int devfreq_enable_event_dev(struct devfreq_event_dev *event_dev)
>>> +{
>>> +	int ret = 0;
>>> +
>>> +	if (!event_dev || !event_dev->desc)
>>> +		return -EINVAL;
>>> +
>>> +	mutex_lock(&event_dev->lock);
>>> +	if (event_dev->desc->ops && event_dev->desc->ops->enable) {
>>> +		ret = event_dev->desc->ops->enable(event_dev);
>>> +		if (ret < 0)
>>> +			goto err;
>>> +	}
>>> +	event_dev->enable_count++;
>>> +err:
>>> +	mutex_unlock(&event_dev->lock);
>>> +
>>> +	return ret;
>>> +}
>>> +EXPORT_SYMBOL_GPL(devfreq_enable_event_dev);
>>> +
>>> +int devfreq_disable_event_dev(struct devfreq_event_dev *event_dev)
>>> +{
>>> +	int ret = 0;
>>> +
>>> +	if (!event_dev || !event_dev->desc)
>>> +		return -EINVAL;
>>> +
>>> +	mutex_lock(&event_dev->lock);
>>> +	if (event_dev->enable_count > 0) {
>>> +		event_dev->enable_count--;
>>> +	} else {
>>> +		dev_warn(&event_dev->dev, "unbalanced enable_count\n");
>>> +		ret = -EINVAL;
>>> +		goto err;
>>> +	}
>>> +
>>> +	if (event_dev->desc->ops && event_dev->desc->ops->disable) {
>>> +		ret = event_dev->desc->ops->disable(event_dev);
>>> +		if (ret < 0) {
>>> +			event_dev->enable_count++;
>>> +			goto err;
>>> +		}
>>> +	}
>>> +err:
>>> +	mutex_unlock(&event_dev->lock);
>>> +
>>> +	return ret;
>>> +}
>>> +EXPORT_SYMBOL_GPL(devfreq_disable_event_dev);
>>> +
>>> +bool devfreq_is_enabled_event_dev(struct devfreq_event_dev *event_dev)
>>> +{
>>> +	bool enabled = false;
>>> +
>>> +	if (!event_dev || !event_dev->desc)
>>> +		return enabled;
>>> +
>>> +	if (!(event_dev->enable_count > 0))
>>> +		return enabled;
>>> +
>>> +	if (event_dev->desc->ops && event_dev->desc->ops->is_enabled)
>>> +		enabled = event_dev->desc->ops->is_enabled(event_dev);
>>> +
>>> +	return enabled;
>>> +}
>>> +EXPORT_SYMBOL_GPL(devfreq_is_enabled_event_dev);
>>> +
>>> +int devfreq_set_event_event_dev(struct devfreq_event_dev *event_dev)
>>
>> The convention of names you use is not obvious to me. I would expect
>> rather devfreq_event_dev_XXX (where XXX is "set_event", "is_enabled"
>> etc).
>> The one above is good example what is the issue with current convention:
>> devfreq_set_event_event_dev
>>             ^     ^
>> This double "event" looks weird.
> 
> You're right. I'll have to fix the function name according to your comment.
> 
>>
>>> +{
>>> +	if (!event_dev || !event_dev->desc)
>>> +		return -EINVAL;
>>> +
>>> +	if (!devfreq_is_enabled_event_dev(event_dev))
>>> +		return -EPERM;
>>> +
>>> +	if (event_dev->desc->ops && event_dev->desc->ops->set_event)
>>> +		return event_dev->desc->ops->set_event(event_dev);
>>
>> No mutexes here? What exactly is protected by mutex?
> 
> I used the mutex when devfreq-event class read/write the data of devfreq_event_dev structure.

I'll use mutex for set_event() function according to your comment.

> 
>>
>>> +
>>> +	return -EINVAL;
>>> +}
>>> +EXPORT_SYMBOL_GPL(devfreq_set_event_event_dev);
>>> +
>>> +int devfreq_get_event_event_dev(struct devfreq_event_dev *event_dev,
>>> +				int *total_event)
>>> +{
>>> +	if (!event_dev || !event_dev->desc)
>>> +		return -EINVAL;
>>> +
>>> +	if (!devfreq_is_enabled_event_dev(event_dev))
>>> +		return -EPERM;
>>> +
>>> +	if (event_dev->desc->ops && event_dev->desc->ops->get_event)
>>> +		return event_dev->desc->ops->get_event(event_dev, total_event);
>>> +
>>> +	return -EINVAL;
>>> +}
>>> +EXPORT_SYMBOL_GPL(devfreq_get_event_event_dev);
>>> +
>>> +int devfreq_reset_event_dev(struct devfreq_event_dev *event_dev)
>>> +{
>>> +	if (!event_dev || !event_dev->desc)
>>> +		return -EINVAL;
>>> +
>>> +	if (!devfreq_is_enabled_event_dev(event_dev))
>>> +		return -EPERM;
>>> +
>>> +	if (event_dev->desc->ops && event_dev->desc->ops->reset)
>>> +		return event_dev->desc->ops->reset(event_dev);
>>
>> Same here, no mutex?
> 
> I replied it on previous your question.

I'll use mutex for set_event() function according to your comment.

Best Regards,
Chanwoo Choi

Re: [RFC PATCHv2 0/7] devfreq: Add devfreq-event class to provide raw data for devfreq device

[Chanwoo Choi]

Hi Krzysztof,

On 12/10/2014 10:21 PM, Chanwoo Choi wrote:
> Hi Krzysztof,
> 
> On Wed, Dec 10, 2014 at 7:07 PM, Krzysztof Kozlowski
> <k.kozlowski@samsung.com> wrote:
>> On wto, 2014-12-09 at 23:12 +0900, Chanwoo Choi wrote:
>>> This patchset add new devfreq_event class to provide raw data to determine
>>> current utilization of device  which is used for devfreq governor.
>>>
>>> [Description of devfreq-event class]
>>> This patchset add new devfreq_event class for devfreq_event device which provide
>>> raw data (e.g., memory bus utilization/GPU utilization). This raw data from
>>> devfreq_event data would be used for the governor of devfreq subsystem.
>>> - devfreq_event device : Provide raw data for governor of existing devfreq device
>>> - devfreq device       : Monitor device state and change frequency/voltage of device
>>>                          using the raw data from devfreq_event device
>>>
>>> The devfreq subsystem support generic DVFS(Dynamic Voltage/Frequency Scaling)
>>> for Non-CPU Devices. The devfreq device would dertermine current device state
>>> using various governor (e.g., ondemand, performance, powersave). After completed
>>> determination of system state, devfreq device would change the frequency/voltage
>>> of devfreq device according to the result of governor.
>>>
>>> But, devfreq governor must need basic data which indicates current device state.
>>> Existing devfreq subsystem only consider devfreq device which check current system
>>> state and determine proper system state using basic data. There is no subsystem
>>> for device providing basic data to devfreq device.
>>>
>>> The devfreq subsystem must need devfreq_event device(data-provider device) for
>>> existing devfreq device. So, this patch add new devfreq_event class for
>>> devfreq_event device which read various basic data(e.g, memory bus utilization,
>>> GPU utilization) and provide measured data to existing devfreq device through
>>> standard APIs of devfreq_event class.
>>>
>>> The following description explains the feature of two kind of devfreq class:
>>> - devfreq class (existing)
>>>  : devfreq consumer device use raw data from devfreq_event device for
>>>    determining proper current system state and change voltage/frequency
>>>    dynamically using various governors.
>>> - devfreq_event class (new)
>>>  : Provide measured raw data to devfreq device for governor
>>>
>>> Also, the devfreq-event device would support various type event as following:
>>>  : DEVFREQ_EVENT_TYPE_RAW_DATA
>>>  : DEVFREQ_EVENT_TYPE_UTILIZATION
>>>  : DEVFREQ_EVENT_TYPE_BANDWIDTH
>>>  : DEVFREQ_EVENT_TYPE_LATENCY
>>>
>>> [For example]
>>> If board dts includes PPMU_DMC0/DMC1/CPU event node,
>>> would show following sysfs entry. Also devfreq driver(e.g., exynos4_bus.c)
>>> can get the instance of devfreq-event device by using provided API and then
>>> get raw data which reflect the current state of device.
>>>
>>
>> Hi,
>>
>> Overall I like the idea. I understand that now devfreq devices (like
>> exynos devfreq) should bind themselves to buses, not to PPMU. It makes
>> sense to me because bus clock and bus voltage are properties of bus, not
>> monitoring unit.
>>
>> I see that this is still a RFC so it would be hard to base current
>> devfreq work on top of it.
> 
> The goal of this patch set is not for devfreq device. This patch set
> just is used for
> devfreq-event device according to patch description.
> 
> The devfreq must need devfreq-event device (e.g., exynos-ppmu.c)
> but current devfreq subsystem have not been supported any reason for
> exyons-ppmu.c as devfreq-event device.
> 
> I discussed this same issue on following patch[1] which was posted to support
> exynos4 busfreq by me.
> [1] https://lkml.org/lkml/2014/3/14/132
> - [PATCHv2 8/8] devfreq: exynos4: Add busfreq driver for exynos4210/exynos4x12
> After discussion, I thought that devfreq-event class must be needed
> before supporting
> the dt of devfreq device.
> 
> So, I think that devfreq have to support the devfreq-event subsystem
> for exynos-ppmu. And then I'll work to support the device-tree of devfreq device
> as next job.
> 
>>
>> One more general comment: you're adding a some API which is not used by
>> current devfreq_event user (exynos). For example the exclusive flag or
>> event types. I think it will be simpler to stick to the basic approach
>> (reduced API). If some other user emerges then new API will be added.
> 
> I think that devfreq-event must support both event_flag(e.g., exclusive flag)
> and event_type(e.g., raw_data, utilization, bandwidth, latency) because
> devfreq-event device (e.g., exynos-ppmu) would provide various event data.
> For example, exynos-ppmu driver could support the utilization/bandwidth/latency
> of each IP of Exynos SoC. I checked PPMU IP on Exynos TRM.
> 
> Also, If specific devfreq-event device should be used on only one device driver,
> devfrewq-event class have to support 'exclusive' flag. If
> devfreq-event class don't
> support the exclusive flag, devfreq-event could not guarantee the
> integrity of event
> from devfreq-event device.

I'll drop 'exclusive' flag according to your comment.

Thanks for your review.

Best Regards,
Chanwoo Choi

Re: [RFC PATCHv2 1/7] devfreq: event: Add new devfreq_event class to provide basic data for devfreq governor

[Krzysztof Kozlowski]

On pią, 2014-12-12 at 12:42 +0900, Chanwoo Choi wrote:
> Hi Krzysztof,
> 
> I replied again this mail because I'll use the mutex for set_event()/get_event()
> according to your comment. But, of_parse_phandle() seems that this function
> don't need the of_node_put() function.
> 
> 
> On 12/11/2014 11:13 AM, Chanwoo Choi wrote:
> > Hi Krzysztof,
> > 
> > First of all, thanks for your review.
> > 
> > On 12/10/2014 06:37 PM, Krzysztof Kozlowski wrote:
> >> On wto, 2014-12-09 at 23:13 +0900, Chanwoo Choi wrote:
> >>> This patch add new devfreq_event class for devfreq_event device which provide
> >>> raw data (e.g., memory bus utilization/GPU utilization). This raw data from
> >>> devfreq_event data would be used for the governor of devfreq subsystem.
> >>> - devfreq_event device : Provide raw data for governor of existing devfreq device
> >>> - devfreq device       : Monitor device state and change frequency/voltage of device
> >>>                          using the raw data from devfreq_event device
> >>>
> >>> The devfreq subsystem support generic DVFS(Dynamic Voltage/Frequency Scaling)
> >>> for Non-CPU Devices. The devfreq device would dertermine current device state
> >>> using various governor (e.g., ondemand, performance, powersave). After completed
> >>> determination of system state, devfreq device would change the frequency/voltage
> >>> of devfreq device according to the result of governor.
> >>>
> >>> But, devfreq governor must need basic data which indicates current device state.
> >>> Existing devfreq subsystem only consider devfreq device which check current system
> >>> state and determine proper system state using basic data. There is no subsystem
> >>> for device providing basic data to devfreq device.
> >>>
> >>> The devfreq subsystem must need devfreq_event device(data-provider device) for
> >>> existing devfreq device. So, this patch add new devfreq_event class for
> >>> devfreq_event device which read various basic data(e.g, memory bus utilization,
> >>> GPU utilization) and provide measured data to existing devfreq device through
> >>> standard APIs of devfreq_event class.
> >>>
> >>> The following description explains the feature of two kind of devfreq class:
> >>> - devfreq class (existing)
> >>>  : devfreq consumer device use raw data from devfreq_event device for
> >>>    determining proper current system state and change voltage/frequency
> >>>    dynamically using various governors.
> >>>
> >>> - devfreq_event class (new)
> >>>  : Provide measured raw data to devfreq device for governor
> >>>
> >>> Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
> >>> Cc: Kyungmin Park <kyungmin.park@samsung.com>
> >>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
> >>> ---
> >>>  drivers/devfreq/Kconfig         |   2 +
> >>>  drivers/devfreq/Makefile        |   5 +-
> >>>  drivers/devfreq/devfreq-event.c | 302 ++++++++++++++++++++++++++++++++++++++++
> >>>  drivers/devfreq/event/Makefile  |   1 +
> >>>  include/linux/devfreq.h         | 141 +++++++++++++++++++
> >>>  5 files changed, 450 insertions(+), 1 deletion(-)
> >>>  create mode 100644 drivers/devfreq/devfreq-event.c
> >>>  create mode 100644 drivers/devfreq/event/Makefile
> >>>
> >>> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
> >>> index faf4e70..4d15b62 100644
> >>> --- a/drivers/devfreq/Kconfig
> >>> +++ b/drivers/devfreq/Kconfig
> >>> @@ -87,4 +87,6 @@ config ARM_EXYNOS5_BUS_DEVFREQ
> >>>  	  It reads PPMU counters of memory controllers and adjusts the
> >>>  	  operating frequencies and voltages with OPP support.
> >>>  
> >>> +comment "DEVFREQ Event Drivers"
> >>> +
> >>>  endif # PM_DEVFREQ
> >>> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
> >>> index 16138c9..a1ffabe 100644
> >>> --- a/drivers/devfreq/Makefile
> >>> +++ b/drivers/devfreq/Makefile
> >>> @@ -1,4 +1,4 @@
> >>> -obj-$(CONFIG_PM_DEVFREQ)	+= devfreq.o
> >>> +obj-$(CONFIG_PM_DEVFREQ)		+= devfreq.o devfreq-event.o
> >>>  obj-$(CONFIG_DEVFREQ_GOV_SIMPLE_ONDEMAND)	+= governor_simpleondemand.o
> >>>  obj-$(CONFIG_DEVFREQ_GOV_PERFORMANCE)	+= governor_performance.o
> >>>  obj-$(CONFIG_DEVFREQ_GOV_POWERSAVE)	+= governor_powersave.o
> >>> @@ -7,3 +7,6 @@ obj-$(CONFIG_DEVFREQ_GOV_USERSPACE)	+= governor_userspace.o
> >>>  # DEVFREQ Drivers
> >>>  obj-$(CONFIG_ARM_EXYNOS4_BUS_DEVFREQ)	+= exynos/
> >>>  obj-$(CONFIG_ARM_EXYNOS5_BUS_DEVFREQ)	+= exynos/
> >>> +
> >>> +# DEVFREQ Event Drivers
> >>> +obj-$(CONFIG_PM_DEVFREQ)		+= event/
> >>> diff --git a/drivers/devfreq/devfreq-event.c b/drivers/devfreq/devfreq-event.c
> >>> new file mode 100644
> >>> index 0000000..b47329f
> >>> --- /dev/null
> >>> +++ b/drivers/devfreq/devfreq-event.c
> >>> @@ -0,0 +1,302 @@
> >>> +/*
> >>> + * devfreq-event: Generic DEVFREQ Event class driver
> >>> + *
> >>> + * Copyright (C) 2014 Samsung Electronics
> >>> + * Chanwoo Choi <cw00.choi@samsung.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.
> >>> + *
> >>> + * This driver is based on drivers/devfreq/devfreq.c
> >>> + */
> >>> +
> >>> +#include <linux/kernel.h>
> >>> +#include <linux/sched.h>
> >>> +#include <linux/errno.h>
> >>> +#include <linux/err.h>
> >>> +#include <linux/init.h>
> >>> +#include <linux/module.h>
> >>> +#include <linux/slab.h>
> >>> +#include <linux/stat.h>
> >>> +#include <linux/pm_opp.h>
> >>> +#include <linux/devfreq.h>
> >>> +#include <linux/workqueue.h>
> >>> +#include <linux/platform_device.h>
> >>> +#include <linux/list.h>
> >>> +#include <linux/printk.h>
> >>> +#include <linux/hrtimer.h>
> >>> +#include <linux/of.h>
> >>> +#include "governor.h"
> >>> +
> >>> +static struct class *devfreq_event_class;
> >>> +
> >>> +/* The list of all devfreq event list */
> >>> +static LIST_HEAD(devfreq_event_list);
> >>> +static DEFINE_MUTEX(devfreq_event_list_lock);
> >>> +
> >>> +#define to_devfreq_event(DEV) container_of(DEV, struct devfreq_event_dev, dev)
> >>> +
> >>> +struct devfreq_event_dev *devfreq_add_event_device(struct device *dev,
> >>> +					struct devfreq_event_desc *desc)
> >>> +{
> >>> +	struct devfreq_event_dev *event_dev;
> >>> +	static atomic_t event_no = ATOMIC_INIT(0);
> >>> +	int ret;
> >>> +
> >>> +	if (!dev || !desc)
> >>> +		return ERR_PTR(-EINVAL);
> >>> +
> >>> +	if (!desc->name || !desc->ops)
> >>> +		return ERR_PTR(-EINVAL);
> >>> +
> >>> +	event_dev = kzalloc(sizeof(struct devfreq_event_dev), GFP_KERNEL);
> >>> +	if (!event_dev)
> >>> +		return ERR_PTR(-ENOMEM);
> >>
> >> Is this memory freed anywhere when driver is removed? I couldn't find
> >> it. I couldn't also find function like devfreq_remove_event_device()
> >> which would be reverting all the work done when adding.
> > 
> > You're right. I'll use devm_kzalloc instead of kzalloc.
> > 
> >>
> >>> +
> >>> +	mutex_lock(&devfreq_event_list_lock);
> >>> +
> >>> +	mutex_init(&event_dev->lock);
> >>> +	event_dev->desc = desc;
> >>> +	event_dev->dev.parent = dev;
> >>> +	event_dev->dev.class = devfreq_event_class;
> >>> +
> >>> +	dev_set_name(&event_dev->dev, "event.%d",
> >>> +		     atomic_inc_return(&event_no) - 1);
> >>> +	ret = device_register(&event_dev->dev);
> >>> +	if (ret != 0) {
> >>> +		put_device(&event_dev->dev);
> >>> +		goto err;
> >>> +	}
> >>> +	dev_set_drvdata(&event_dev->dev, event_dev);
> >>> +
> >>> +	/* Add devfreq event device to devfreq_event_list */
> >>> +	INIT_LIST_HEAD(&event_dev->node);
> >>> +	list_add(&event_dev->node, &devfreq_event_list);
> >>> +
> >>> +	mutex_unlock(&devfreq_event_list_lock);
> >>> +
> >>> +	return event_dev;
> >>> +err:
> >>
> >> Missing 'mutex_unlock(&devfreq_event_list_lock)' here.
> > 
> > OK. I'll fix it.
> > 
> >>
> >>
> >>> +	kfree(event_dev);
> >>> +	return ERR_PTR(ret);
> >>> +}
> >>> +EXPORT_SYMBOL(devfreq_add_event_device);
> >>> +
> >>> +struct devfreq_event_dev *devfreq_get_event_dev(const char *event_dev_name)
> >>> +{
> >>> +	struct devfreq_event_dev *event_dev;
> >>> +
> >>> +	mutex_lock(&devfreq_event_list_lock);
> >>> +	list_for_each_entry(event_dev, &devfreq_event_list, node) {
> >>> +		if (!strcmp(event_dev->desc->name, event_dev_name))
> >>> +			goto out;
> >>> +	}
> >>> +	event_dev = NULL;
> >>> +out:
> >>> +	mutex_unlock(&devfreq_event_list_lock);
> >>> +
> >>> +	return event_dev;
> >>> +}
> >>> +EXPORT_SYMBOL_GPL(devfreq_get_event_dev);
> >>> +
> >>> +struct devfreq_event_dev *devfreq_get_event_dev_by_phandle(struct device *dev,
> >>> +						      int index)
> >>> +{
> >>> +	struct device_node *node;
> >>> +	struct devfreq_event_dev *event_dev;
> >>> +
> >>> +	if (!dev->of_node) {
> >>> +		dev_err(dev, "device does not have a device node entry\n");
> >>> +		return ERR_PTR(-EINVAL);
> >>> +	}
> >>> +
> >>> +	node = of_parse_phandle(dev->of_node, "devfreq-events", index);
> >>> +	if (!node) {
> >>> +		dev_err(dev, "failed to get phandle in %s node\n",
> >>> +			dev->of_node->full_name);
> >>> +		return ERR_PTR(-ENODEV);
> >>> +	}
> >>> +
> >>> +	event_dev = devfreq_get_event_dev(node->name);
> >>> +	if (!event_dev) {
> >>> +		dev_err(dev, "unable to get devfreq-event device : %s\n",
> >>> +			node->name);
> >>
> >> of_node_put() for node obtained with of_parse_phandle().
> > 
> > OK. I'll add it.
> 
> of_parse_phandle() seems that it don't need of_node_put().

The of_parse_phandle() increases the refcount for node it returns.
>From documentation:

/**
 * Returns the device_node pointer with refcount incremented.  Use
 * of_node_put() on it when done.
 */

The function returns error condition but node was found and its refcnt
was incremented. Then why do you think that of_node_put() is not
necessary here?

> 
> > 
> >>
> >>> +		return ERR_PTR(-ENODEV);
> >>> +	}
> >>> +
> >>> +	return event_dev;
> >>> +}
> >>> +EXPORT_SYMBOL_GPL(devfreq_get_event_dev_by_phandle);
> >>> +
> >>> +int devfreq_put_event_dev(struct devfreq_event_dev *event_dev)
> >>> +{
> >>
> >> of_node_put() here to decrement refcnt from of_parse_phandle()?
> > 
> > It is my mistake. I think that devfreq_put_event_dev is not necesssary.
> > I'll remove it.

Then how to decrease the refcnt for node obtained in
devfreq_get_event_dev_by_phandle()?

Best regards,
Krzysztof

Re: [RFC PATCHv2 1/7] devfreq: event: Add new devfreq_event class to provide basic data for devfreq governor

[Chanwoo Choi]

Hi Krzysztof,

On 12/15/2014 07:30 PM, Krzysztof Kozlowski wrote:
> On pią, 2014-12-12 at 12:42 +0900, Chanwoo Choi wrote:
>> Hi Krzysztof,
>>
>> I replied again this mail because I'll use the mutex for set_event()/get_event()
>> according to your comment. But, of_parse_phandle() seems that this function
>> don't need the of_node_put() function.
>>
>>
>> On 12/11/2014 11:13 AM, Chanwoo Choi wrote:
>>> Hi Krzysztof,
>>>
>>> First of all, thanks for your review.
>>>
>>> On 12/10/2014 06:37 PM, Krzysztof Kozlowski wrote:
>>>> On wto, 2014-12-09 at 23:13 +0900, Chanwoo Choi wrote:
>>>>> This patch add new devfreq_event class for devfreq_event device which provide
>>>>> raw data (e.g., memory bus utilization/GPU utilization). This raw data from
>>>>> devfreq_event data would be used for the governor of devfreq subsystem.
>>>>> - devfreq_event device : Provide raw data for governor of existing devfreq device
>>>>> - devfreq device       : Monitor device state and change frequency/voltage of device
>>>>>                          using the raw data from devfreq_event device
>>>>>
>>>>> The devfreq subsystem support generic DVFS(Dynamic Voltage/Frequency Scaling)
>>>>> for Non-CPU Devices. The devfreq device would dertermine current device state
>>>>> using various governor (e.g., ondemand, performance, powersave). After completed
>>>>> determination of system state, devfreq device would change the frequency/voltage
>>>>> of devfreq device according to the result of governor.
>>>>>
>>>>> But, devfreq governor must need basic data which indicates current device state.
>>>>> Existing devfreq subsystem only consider devfreq device which check current system
>>>>> state and determine proper system state using basic data. There is no subsystem
>>>>> for device providing basic data to devfreq device.
>>>>>
>>>>> The devfreq subsystem must need devfreq_event device(data-provider device) for
>>>>> existing devfreq device. So, this patch add new devfreq_event class for
>>>>> devfreq_event device which read various basic data(e.g, memory bus utilization,
>>>>> GPU utilization) and provide measured data to existing devfreq device through
>>>>> standard APIs of devfreq_event class.
>>>>>
>>>>> The following description explains the feature of two kind of devfreq class:
>>>>> - devfreq class (existing)
>>>>>  : devfreq consumer device use raw data from devfreq_event device for
>>>>>    determining proper current system state and change voltage/frequency
>>>>>    dynamically using various governors.
>>>>>
>>>>> - devfreq_event class (new)
>>>>>  : Provide measured raw data to devfreq device for governor
>>>>>
>>>>> Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
>>>>> Cc: Kyungmin Park <kyungmin.park@samsung.com>
>>>>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>>>>> ---
>>>>>  drivers/devfreq/Kconfig         |   2 +
>>>>>  drivers/devfreq/Makefile        |   5 +-
>>>>>  drivers/devfreq/devfreq-event.c | 302 ++++++++++++++++++++++++++++++++++++++++
>>>>>  drivers/devfreq/event/Makefile  |   1 +
>>>>>  include/linux/devfreq.h         | 141 +++++++++++++++++++
>>>>>  5 files changed, 450 insertions(+), 1 deletion(-)
>>>>>  create mode 100644 drivers/devfreq/devfreq-event.c
>>>>>  create mode 100644 drivers/devfreq/event/Makefile
>>>>>
>>>>> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
>>>>> index faf4e70..4d15b62 100644
>>>>> --- a/drivers/devfreq/Kconfig
>>>>> +++ b/drivers/devfreq/Kconfig
>>>>> @@ -87,4 +87,6 @@ config ARM_EXYNOS5_BUS_DEVFREQ
>>>>>  	  It reads PPMU counters of memory controllers and adjusts the
>>>>>  	  operating frequencies and voltages with OPP support.
>>>>>  
>>>>> +comment "DEVFREQ Event Drivers"
>>>>> +
>>>>>  endif # PM_DEVFREQ
>>>>> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
>>>>> index 16138c9..a1ffabe 100644
>>>>> --- a/drivers/devfreq/Makefile
>>>>> +++ b/drivers/devfreq/Makefile
>>>>> @@ -1,4 +1,4 @@
>>>>> -obj-$(CONFIG_PM_DEVFREQ)	+= devfreq.o
>>>>> +obj-$(CONFIG_PM_DEVFREQ)		+= devfreq.o devfreq-event.o
>>>>>  obj-$(CONFIG_DEVFREQ_GOV_SIMPLE_ONDEMAND)	+= governor_simpleondemand.o
>>>>>  obj-$(CONFIG_DEVFREQ_GOV_PERFORMANCE)	+= governor_performance.o
>>>>>  obj-$(CONFIG_DEVFREQ_GOV_POWERSAVE)	+= governor_powersave.o
>>>>> @@ -7,3 +7,6 @@ obj-$(CONFIG_DEVFREQ_GOV_USERSPACE)	+= governor_userspace.o
>>>>>  # DEVFREQ Drivers
>>>>>  obj-$(CONFIG_ARM_EXYNOS4_BUS_DEVFREQ)	+= exynos/
>>>>>  obj-$(CONFIG_ARM_EXYNOS5_BUS_DEVFREQ)	+= exynos/
>>>>> +
>>>>> +# DEVFREQ Event Drivers
>>>>> +obj-$(CONFIG_PM_DEVFREQ)		+= event/
>>>>> diff --git a/drivers/devfreq/devfreq-event.c b/drivers/devfreq/devfreq-event.c
>>>>> new file mode 100644
>>>>> index 0000000..b47329f
>>>>> --- /dev/null
>>>>> +++ b/drivers/devfreq/devfreq-event.c
>>>>> @@ -0,0 +1,302 @@
>>>>> +/*
>>>>> + * devfreq-event: Generic DEVFREQ Event class driver
>>>>> + *
>>>>> + * Copyright (C) 2014 Samsung Electronics
>>>>> + * Chanwoo Choi <cw00.choi@samsung.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.
>>>>> + *
>>>>> + * This driver is based on drivers/devfreq/devfreq.c
>>>>> + */
>>>>> +
>>>>> +#include <linux/kernel.h>
>>>>> +#include <linux/sched.h>
>>>>> +#include <linux/errno.h>
>>>>> +#include <linux/err.h>
>>>>> +#include <linux/init.h>
>>>>> +#include <linux/module.h>
>>>>> +#include <linux/slab.h>
>>>>> +#include <linux/stat.h>
>>>>> +#include <linux/pm_opp.h>
>>>>> +#include <linux/devfreq.h>
>>>>> +#include <linux/workqueue.h>
>>>>> +#include <linux/platform_device.h>
>>>>> +#include <linux/list.h>
>>>>> +#include <linux/printk.h>
>>>>> +#include <linux/hrtimer.h>
>>>>> +#include <linux/of.h>
>>>>> +#include "governor.h"
>>>>> +
>>>>> +static struct class *devfreq_event_class;
>>>>> +
>>>>> +/* The list of all devfreq event list */
>>>>> +static LIST_HEAD(devfreq_event_list);
>>>>> +static DEFINE_MUTEX(devfreq_event_list_lock);
>>>>> +
>>>>> +#define to_devfreq_event(DEV) container_of(DEV, struct devfreq_event_dev, dev)
>>>>> +
>>>>> +struct devfreq_event_dev *devfreq_add_event_device(struct device *dev,
>>>>> +					struct devfreq_event_desc *desc)
>>>>> +{
>>>>> +	struct devfreq_event_dev *event_dev;
>>>>> +	static atomic_t event_no = ATOMIC_INIT(0);
>>>>> +	int ret;
>>>>> +
>>>>> +	if (!dev || !desc)
>>>>> +		return ERR_PTR(-EINVAL);
>>>>> +
>>>>> +	if (!desc->name || !desc->ops)
>>>>> +		return ERR_PTR(-EINVAL);
>>>>> +
>>>>> +	event_dev = kzalloc(sizeof(struct devfreq_event_dev), GFP_KERNEL);
>>>>> +	if (!event_dev)
>>>>> +		return ERR_PTR(-ENOMEM);
>>>>
>>>> Is this memory freed anywhere when driver is removed? I couldn't find
>>>> it. I couldn't also find function like devfreq_remove_event_device()
>>>> which would be reverting all the work done when adding.
>>>
>>> You're right. I'll use devm_kzalloc instead of kzalloc.
>>>
>>>>
>>>>> +
>>>>> +	mutex_lock(&devfreq_event_list_lock);
>>>>> +
>>>>> +	mutex_init(&event_dev->lock);
>>>>> +	event_dev->desc = desc;
>>>>> +	event_dev->dev.parent = dev;
>>>>> +	event_dev->dev.class = devfreq_event_class;
>>>>> +
>>>>> +	dev_set_name(&event_dev->dev, "event.%d",
>>>>> +		     atomic_inc_return(&event_no) - 1);
>>>>> +	ret = device_register(&event_dev->dev);
>>>>> +	if (ret != 0) {
>>>>> +		put_device(&event_dev->dev);
>>>>> +		goto err;
>>>>> +	}
>>>>> +	dev_set_drvdata(&event_dev->dev, event_dev);
>>>>> +
>>>>> +	/* Add devfreq event device to devfreq_event_list */
>>>>> +	INIT_LIST_HEAD(&event_dev->node);
>>>>> +	list_add(&event_dev->node, &devfreq_event_list);
>>>>> +
>>>>> +	mutex_unlock(&devfreq_event_list_lock);
>>>>> +
>>>>> +	return event_dev;
>>>>> +err:
>>>>
>>>> Missing 'mutex_unlock(&devfreq_event_list_lock)' here.
>>>
>>> OK. I'll fix it.
>>>
>>>>
>>>>
>>>>> +	kfree(event_dev);
>>>>> +	return ERR_PTR(ret);
>>>>> +}
>>>>> +EXPORT_SYMBOL(devfreq_add_event_device);
>>>>> +
>>>>> +struct devfreq_event_dev *devfreq_get_event_dev(const char *event_dev_name)
>>>>> +{
>>>>> +	struct devfreq_event_dev *event_dev;
>>>>> +
>>>>> +	mutex_lock(&devfreq_event_list_lock);
>>>>> +	list_for_each_entry(event_dev, &devfreq_event_list, node) {
>>>>> +		if (!strcmp(event_dev->desc->name, event_dev_name))
>>>>> +			goto out;
>>>>> +	}
>>>>> +	event_dev = NULL;
>>>>> +out:
>>>>> +	mutex_unlock(&devfreq_event_list_lock);
>>>>> +
>>>>> +	return event_dev;
>>>>> +}
>>>>> +EXPORT_SYMBOL_GPL(devfreq_get_event_dev);
>>>>> +
>>>>> +struct devfreq_event_dev *devfreq_get_event_dev_by_phandle(struct device *dev,
>>>>> +						      int index)
>>>>> +{
>>>>> +	struct device_node *node;
>>>>> +	struct devfreq_event_dev *event_dev;
>>>>> +
>>>>> +	if (!dev->of_node) {
>>>>> +		dev_err(dev, "device does not have a device node entry\n");
>>>>> +		return ERR_PTR(-EINVAL);
>>>>> +	}
>>>>> +
>>>>> +	node = of_parse_phandle(dev->of_node, "devfreq-events", index);
>>>>> +	if (!node) {
>>>>> +		dev_err(dev, "failed to get phandle in %s node\n",
>>>>> +			dev->of_node->full_name);
>>>>> +		return ERR_PTR(-ENODEV);
>>>>> +	}
>>>>> +
>>>>> +	event_dev = devfreq_get_event_dev(node->name);
>>>>> +	if (!event_dev) {
>>>>> +		dev_err(dev, "unable to get devfreq-event device : %s\n",
>>>>> +			node->name);
>>>>
>>>> of_node_put() for node obtained with of_parse_phandle().
>>>
>>> OK. I'll add it.
>>
>> of_parse_phandle() seems that it don't need of_node_put().
> 
> The of_parse_phandle() increases the refcount for node it returns.
>>From documentation:
> 
> /**
>  * Returns the device_node pointer with refcount incremented.  Use
>  * of_node_put() on it when done.
>  */
> 
> The function returns error condition but node was found and its refcnt
> was incremented. Then why do you think that of_node_put() is not
> necessary here?

OK, I'll call of_node_put() as following:

diff --git a/drivers/devfreq/devfreq-event.c b/drivers/devfreq/devfreq-event.c
index cd9c485..a1edafc 100644
--- a/drivers/devfreq/devfreq-event.c
+++ b/drivers/devfreq/devfreq-event.c
@@ -282,9 +282,12 @@ out:
        if (!edev) {
                dev_err(dev, "unable to get devfreq-event device : %s\n",
                        node->name);
+               of_node_put(node);
                return ERR_PTR(-ENODEV);
        }
 
+       of_node_put(node);
+
        return edev;
 }

> 
>>
>>>
>>>>
>>>>> +		return ERR_PTR(-ENODEV);
>>>>> +	}
>>>>> +
>>>>> +	return event_dev;
>>>>> +}
>>>>> +EXPORT_SYMBOL_GPL(devfreq_get_event_dev_by_phandle);
>>>>> +
>>>>> +int devfreq_put_event_dev(struct devfreq_event_dev *event_dev)
>>>>> +{
>>>>
>>>> of_node_put() here to decrement refcnt from of_parse_phandle()?
>>>
>>> It is my mistake. I think that devfreq_put_event_dev is not necesssary.
>>> I'll remove it.
> 
> Then how to decrease the refcnt for node obtained in
> devfreq_get_event_dev_by_phandle()?

I'll call of_node_put in devfreq_event_get_edev_by_phandle() according to your comment.

Thanks for your review.

Best Regards,
Chanwoo Choi