public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* Re: Re: [PATCHv8 1/9] devfreq: event: Add new devfreq_event class to provide basic data for devfreq governor
@ 2015-01-20  6:59 MyungJoo Ham
  2015-01-20  7:25 ` Chanwoo Choi
  0 siblings, 1 reply; 6+ messages in thread
From: MyungJoo Ham @ 2015-01-20  6:59 UTC (permalink / raw)
  To: 최찬우
  Cc: kgene@kernel.org, 박경민,
	rafael.j.wysocki@intel.com, mark.rutland@arm.com,
	ABHILASH KESAVAN, tomasz.figa@gmail.com, Krzysztof Kozlowski,
	Bartlomiej Zolnierkiewicz, robh+dt@kernel.org,
	대인기, linux-pm@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-samsung-soc@vger.kernel.org

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=utf-8, Size: 5027 bytes --]

>  
> Dear Myungjoo,
>
>On 01/20/2015 01:34 PM, MyungJoo Ham wrote:
>>>   
[]
>>> +
>>> +	mutex_lock(&edev->lock);
>>> +	if (edev->desc->ops && edev->desc->ops->enable) {
>>> +		ret = edev->desc->ops->enable(edev);
>>> +		if (ret < 0)
>>> +			goto err;
>>> +	}
>> 
>> Is there any reason to call enable(edev) even when enable_count is already > 0 
>> while you do not call disable(edev) while enable_count > 0?
>> 
>> I think this may incur errors in the related device drivers.
>> (e.g., incorrect pairing of clk/runtime-pm/regulator enable/disable
>> at the device driver side)
>
>You're right. This part has potential errors. I'll fix it as following:
>If edev is already enabled, devfreq_event_enable_edev() will just return
>without any operation because devfreq-event(edev) can handle only one event
>at the same time.
> 
>	mutex_lock(&edev->lock);
>	if (edev->enable_count)
>		dev_warn(&edev->dev, "%s is already enabled\n", edev->desc->name);
>		ret = -EINVAL;
>		goto err;
>	}
>
>	if (edev->desc->ops && edev->desc->ops->enable) {		
>		ret = edev->desc->ops->enable(edev);
>		if (ret < 0)
>			goto err;
>	}
>	edev->enable_count++;

No, your suggested modification creates another bug.

It should not emit "warn" when enable_count > 0 at enable().
It is a natural behavior from drivers.
- You may have multiple drivers using edev.
- You may have multiple threads using edev.

Thus, the above 12 lines should be replaced with:

	if (edev->desc->ops && edev->desc->ops->enable &&
	    edev->enable_count == 0) {
		ret = edev->desc->ops->enable(edev);
		if (ret < 0)
			goto err;
	}
	edev->enable_count++;

>	
>
>> 
>>> +	edev->enable_count++;
>>> +err:
>>> +	mutex_unlock(&edev->lock);
>>> +
>>> +	return ret;
>>> +}
>>> +EXPORT_SYMBOL_GPL(devfreq_event_enable_edev);
>>> +
>>> +/**
>>> + * devfreq_event_disable_edev() - Disable the devfreq-event dev and decrease
>>> + *				  the enable_count of the devfreq-event dev.
>>> + * @edev	: the devfreq-event device
>>> + *
>>> + * Note that this function decrease the enable_count and disable the
>>> + * devfreq-event device. After the devfreq-event device is disabled,
>>> + * devfreq device can't use the devfreq-event device for get/set/reset
>>> + * operations.
>>> + */
>>> +int devfreq_event_disable_edev(struct devfreq_event_dev *edev)
>>> +{
>>> +	int ret = 0;
>>> +
>>> +	if (!edev || !edev->desc)
>>> +		return -EINVAL;
>>> +
>>> +	mutex_lock(&edev->lock);
>>> +	if (edev->enable_count > 0) {
>>> +		edev->enable_count--;
>>> +	} else {
>>> +		dev_warn(&edev->dev, "unbalanced enable_count\n");
>>> +		ret = -EINVAL;
>>> +		goto err;
>>> +	}
>>> +
>>> +	if (edev->desc->ops && edev->desc->ops->disable) {
>>> +		ret = edev->desc->ops->disable(edev);
>>> +		if (ret < 0) {
>>> +			edev->enable_count++;
>>> +			goto err;
>>> +		}

Anyway, have you seen other subsystems doing fall-back operations as you've
done by "edev->enable_count++" here? Or is this your own idea on falling back
from errors with a disable callback?

>>> +	}
>> 
>> You did it correctly with disable here;
>> not calling it when it is not required.

Uh..yeah.. the original patch was incorrect..

>
>As I explained, I'll fix it as following:
>
>	mutex_lock(&edev->lock);
>	if (!edev->enable_count) {
>		dev_warn(&edev->dev, "%s is already disabled\n", edev->desc->name);
>		ret = -EINVAL;
>		goto err;
>	}
>
>	if (edev->desc->ops && edev->desc->ops->disable) {
>		ret = edev->desc->ops->disable(edev);
>		if (ret < 0)
>			goto err;		
>	}
>	edev->enable_count--;

Uh.... I'd say it is still incorrect.

	mutex_lock(&edev->lock);
	if (!edev->enable_count) {
		dev_warn(&edev->dev, "%s is already disabled\n", edev->desc->name);
		ret = -EINVAL;
		goto err;
	}

	edev->enable_count--;
	if (edev->desc->ops && edev->desc->ops->disable &&
	    !edev->enable_count) {
		ret = edev->desc->ops->disable(edev);
		if (ret < 0)
			goto err;		
	}


>
>> 
>>> +err:
>>> +	mutex_unlock(&edev->lock);
>>> +
>>> +	return ret;
>>> +}
>>> +EXPORT_SYMBOL_GPL(devfreq_event_disable_edev);
>>> +
>> 
>> []
>>> +EXPORT_SYMBOL_GPL(devfreq_event_is_enabled);
>> []
>> 
>>> +EXPORT_SYMBOL_GPL(devfreq_event_set_event);
>> []
>> 
[]
>>> +int devfreq_event_reset_event(struct devfreq_event_dev *edev)
>>> +{
>>> +	int ret = 0;
>>> +
>>> +	if (!edev || !edev->desc)
>>> +		return -EINVAL;
>>> +
>>> +	if (!devfreq_event_is_enabled(edev))
>>> +		return -EPERM;
>>> +
>>> +	mutex_lock(&edev->lock);
>>> +	if (edev->desc->ops && edev->desc->ops->reset)
>>> +		ret = edev->desc->ops->reset(edev);
>>> +	mutex_unlock(&edev->lock);
>> 
>> In the context of the get_event() handling "load",
>> aren't you supposed to set total_event = event = 0; here?
>
>But, devfreq_event_reset_event() function cannot handle edata instance
>because edata is not included in edev. The edata instance is only used in devfreq_event_get_event().

Ah.. ok then.

>
[]

Cheers,
MyungJoo
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCHv8 1/9] devfreq: event: Add new devfreq_event class to provide basic data for devfreq governor
@ 2015-01-20  4:34 MyungJoo Ham
  2015-01-20  5:29 ` Chanwoo Choi
  0 siblings, 1 reply; 6+ messages in thread
From: MyungJoo Ham @ 2015-01-20  4:34 UTC (permalink / raw)
  To: 최찬우, kgene@kernel.org
  Cc: 박경민, rafael.j.wysocki@intel.com,
	mark.rutland@arm.com, ABHILASH KESAVAN, tomasz.figa@gmail.com,
	Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz,
	robh+dt@kernel.org, 대인기,
	linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-samsung-soc@vger.kernel.org

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=utf-8, Size: 13286 bytes --]

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

[]

> +/**
> + * devfreq_event_enable_edev() - Enable the devfreq-event dev and increase
> + *				 the enable_count of devfreq-event dev.
> + * @edev	: the devfreq-event device
> + *
> + * Note that this function increase the enable_count and enable the
> + * devfreq-event device. The devfreq-event device should be enabled before
> + * using it by devfreq device.
> + */
> +int devfreq_event_enable_edev(struct devfreq_event_dev *edev)
> +{
> +	int ret = 0;
> +
> +	if (!edev || !edev->desc)
> +		return -EINVAL;
> +
> +	mutex_lock(&edev->lock);
> +	if (edev->desc->ops && edev->desc->ops->enable) {
> +		ret = edev->desc->ops->enable(edev);
> +		if (ret < 0)
> +			goto err;
> +	}

Is there any reason to call enable(edev) even when enable_count is already > 0 
while you do not call disable(edev) while enable_count > 0?

I think this may incur errors in the related device drivers.
(e.g., incorrect pairing of clk/runtime-pm/regulator enable/disable
at the device driver side)

> +	edev->enable_count++;
> +err:
> +	mutex_unlock(&edev->lock);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(devfreq_event_enable_edev);
> +
> +/**
> + * devfreq_event_disable_edev() - Disable the devfreq-event dev and decrease
> + *				  the enable_count of the devfreq-event dev.
> + * @edev	: the devfreq-event device
> + *
> + * Note that this function decrease the enable_count and disable the
> + * devfreq-event device. After the devfreq-event device is disabled,
> + * devfreq device can't use the devfreq-event device for get/set/reset
> + * operations.
> + */
> +int devfreq_event_disable_edev(struct devfreq_event_dev *edev)
> +{
> +	int ret = 0;
> +
> +	if (!edev || !edev->desc)
> +		return -EINVAL;
> +
> +	mutex_lock(&edev->lock);
> +	if (edev->enable_count > 0) {
> +		edev->enable_count--;
> +	} else {
> +		dev_warn(&edev->dev, "unbalanced enable_count\n");
> +		ret = -EINVAL;
> +		goto err;
> +	}
> +
> +	if (edev->desc->ops && edev->desc->ops->disable) {
> +		ret = edev->desc->ops->disable(edev);
> +		if (ret < 0) {
> +			edev->enable_count++;
> +			goto err;
> +		}
> +	}

You did it correctly with disable here;
not calling it when it is not required.

> +err:
> +	mutex_unlock(&edev->lock);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(devfreq_event_disable_edev);
> +

[]
> +EXPORT_SYMBOL_GPL(devfreq_event_is_enabled);
[]

> +EXPORT_SYMBOL_GPL(devfreq_event_set_event);
[]

> +
> +/**
> + * devfreq_event_get_event() - Get event and total_event from devfreq-event dev.
> + * @edev	: the devfreq-event device
> + * @edata	: the calculated data of devfreq-event device
> + *
> + * Note that this function get the calculated event data from devfreq-event dev
> + * after stoping the progress of whole sequence of devfreq-event dev.
> + */
> +int devfreq_event_get_event(struct devfreq_event_dev *edev,
> +			    struct devfreq_event_data *edata)
> +{
> +	int ret;
> +
> +	if (!edev || !edev->desc)
> +		return -EINVAL;
> +
> +	if (!edev->desc->ops || !edev->desc->ops->get_event)
> +		return -EINVAL;
> +
> +	if (!devfreq_event_is_enabled(edev))
> +		return -EINVAL;
> +
> +	edata->event = edata->total_event = 0;
> +
> +	mutex_lock(&edev->lock);
> +	ret = edev->desc->ops->get_event(edev, edata);
> +	mutex_unlock(&edev->lock);
> +
> +	if ((edata->total_event <= 0)
> +		|| (edata->event > edata->total_event)) {
> +		edata->event = edata->total_event = 0;
> +		ret = -EINVAL;

total_event is unsigned. (cannot be < 0)

Plus, get_event() may already have returned a negative value with
the intention of giving a error different from EINVAL.

I would just simply set total_event = event = 0 if ret < 0.


> +	}
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(devfreq_event_get_event);
> +
> +/**
> + * devfreq_event_reset_event() - Reset all opeations of devfreq-event dev.
> + * @edev	: the devfreq-event device
> + *
> + * Note that this function stop all operations of devfreq-event dev and reset
> + * the current event data to make the devfreq-event device into initial state.
> + */
> +int devfreq_event_reset_event(struct devfreq_event_dev *edev)
> +{
> +	int ret = 0;
> +
> +	if (!edev || !edev->desc)
> +		return -EINVAL;
> +
> +	if (!devfreq_event_is_enabled(edev))
> +		return -EPERM;
> +
> +	mutex_lock(&edev->lock);
> +	if (edev->desc->ops && edev->desc->ops->reset)
> +		ret = edev->desc->ops->reset(edev);
> +	mutex_unlock(&edev->lock);

In the context of the get_event() handling "load",
aren't you supposed to set total_event = event = 0; here?

> +
> +	if (ret < 0)
> +		return -EINVAL;
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(devfreq_event_reset_event);
> +
> +/**
> + * devfreq_event_get_drvdata() - Return driver-data of devfreq-event dev.
> + * @edev	: the devfreq-event device
> + *
> + * Note that this function return the driver-data of devfreq-event device.
> + */
> +void *devfreq_event_get_drvdata(struct devfreq_event_dev *edev)
> +{
> +	return edev->desc->driver_data;
> +}
> +EXPORT_SYMBOL_GPL(devfreq_event_get_drvdata);

Looks like you can simply write this in the header file.
(either #define or static inline function)

[]

> +EXPORT_SYMBOL_GPL(devfreq_event_get_edev_by_phandle);
[]
> +EXPORT_SYMBOL_GPL(devfreq_event_get_edev_count);
[]

> +/**
> + * devfreq_event_add_edev() - Add new devfreq-event device.
> + * @dev		: the device owning the devfreq-event device being created
> + * @desc	: the devfreq-event device's decriptor which include essential
> + *		  data for devfreq-event device.
> + *
> + * Note that this function add new devfreq-event device to devfreq-event class
> + * list and register the device of the devfreq-event device.
> + */
> +struct devfreq_event_dev *devfreq_event_add_edev(struct device *dev,
> +						struct devfreq_event_desc *desc)
> +{
> +	struct devfreq_event_dev *edev;
> +	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);
> +
> +	if (!desc->ops->set_event || !desc->ops->get_event)
> +		return ERR_PTR(-EINVAL);
> +
> +	edev = kzalloc(sizeof(struct devfreq_event_dev), GFP_KERNEL);

devm_* here? (seeing no free)

> +	if (!edev)
> +		return ERR_PTR(-ENOMEM);
> +
> +	mutex_init(&edev->lock);
> +	edev->desc = desc;
> +	edev->dev.parent = dev;
> +	edev->dev.class = devfreq_event_class;
> +	edev->dev.release = devfreq_event_release_edev;
> +
> +	dev_set_name(&edev->dev, "event.%d", atomic_inc_return(&event_no) - 1);
> +	ret = device_register(&edev->dev);
> +	if (ret < 0) {
> +		put_device(&edev->dev);
> +		return ERR_PTR(ret);
> +	}
> +	dev_set_drvdata(&edev->dev, edev);
> +
> +	INIT_LIST_HEAD(&edev->node);
> +
> +	mutex_lock(&devfreq_event_list_lock);
> +	list_add(&edev->node, &devfreq_event_list);
> +	mutex_unlock(&devfreq_event_list_lock);
> +
> +	return edev;
> +}
> +EXPORT_SYMBOL_GPL(devfreq_event_add_edev);
> +
> +/**
> + * devfreq_event_remove_edev() - Remove the devfreq-event device registered.
> + * @dev		: the devfreq-event device
> + *
> + * Note that this function remove the registered devfreq-event device.
> + */
> +int devfreq_event_remove_edev(struct devfreq_event_dev *edev)
> +{
> +	if (!edev)
> +		return -EINVAL;
> +
> +	mutex_lock(&devfreq_event_list_lock);
> +	WARN_ON(edev->enable_count);

Let's WARN before mutex_lock();

> +	list_del(&edev->node);
> +	device_unregister(&edev->dev);

Let's unregister after mutex_unlock();

The two lines do not need to be protected by the mutex.

> +	mutex_unlock(&devfreq_event_list_lock);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(devfreq_event_remove_edev);
> +

[]

> diff --git a/drivers/devfreq/event/Kconfig b/drivers/devfreq/event/Kconfig
> new file mode 100644
> index 0000000..1ced42c
> --- /dev/null
> +++ b/drivers/devfreq/event/Kconfig
> @@ -0,0 +1,16 @@
> +menuconfig PM_DEVFREQ_EVENT
> +	bool "DEVFREQ-Event device Support"
> +	help
> +	  The devfreq-event device provide the raw data and events which
> +	  indicate the current state of devfreq-event device. The provided
> +	  data from devfreq-event device is used to monitor the state of
> +	  device and determine the suitable size of resource to reduce the
> +	  wasted resource.
> +
> +	  The devfreq-event device can support the various type of events
> +	  (e.g., raw data, utilization, latency, bandwidth). The events
> +	  may be used by devfreq governor and other subsystem.
> +
> +if PM_DEVFREQ_EVENT
> +
> +endif # PM_DEVFREQ_EVENT
> 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-event.h b/include/linux/devfreq-event.h
> new file mode 100644
> index 0000000..b7363f5
> --- /dev/null
> +++ b/include/linux/devfreq-event.h
> @@ -0,0 +1,170 @@
> +/*
> + * devfreq-event: a framework to provide raw data and events of devfreq devices
> + *
> + * Copyright (C) 2014 Samsung Electronics
> + * 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.
> + */
> +
> +#ifndef __LINUX_DEVFREQ_EVENT_H__
> +#define __LINUX_DEVFREQ_EVENT_H__
> +
> +#include <linux/device.h>
> +
> +/**
> + * 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_data - the devfreq-event data
> + *
> + * @event	: the load of devfreq-event device for polling period
> + * @total_event	: the total load of devfreq-event device for polling period
> + *
> + * This structure contains the data of devfreq-event device for polling period.
> + */
> +struct devfreq_event_data {
> +	unsigned long event;
> +	unsigned long total_event;
> +};

I would like to rephrase and rename as follows:

+ * @load_count		: load count of devfreq-event device for the given period.
+ * @total_count	: total count of devfreq-event device for the given period.
+ *			each count may represent a clock cycle,
+ *			a time unit (ns/us/...), or anything the device driver wants.
+ *			Generally, utilization is load_count / total_count.

[]


You may already contact me with synchronous methods for more
prompt discussion. :) Your office is not that far from mine.
(Or during lunch, dinner, or beer sessions as well)


Cheers,
MyungJoo

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCHv8 0/9] devfreq: Add devfreq-event class to provide raw data for devfreq device
@ 2015-01-12 12:34 Chanwoo Choi
  2015-01-12 12:34 ` [PATCHv8 1/9] devfreq: event: Add new devfreq_event class to provide basic data for devfreq governor Chanwoo Choi
  0 siblings, 1 reply; 6+ messages in thread
From: Chanwoo Choi @ 2015-01-12 12:34 UTC (permalink / raw)
  To: myungjoo.ham, kgene
  Cc: kyungmin.park, rafael.j.wysocki, mark.rutland, a.kesavan,
	tomasz.figa, k.kozlowski, b.zolnierkie, robh+dt, cw00.choi,
	inki.dae, linux-pm, linux-kernel, linux-arm-kernel,
	linux-samsung-soc

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

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

---------
Changelog:

Changes from v7:
(https://lkml.org/lkml/2015/1/7/795)
- Use EXPORT_SYMBOL_GPL macro instead of EXPORT_SYMBOL
- Fix the binding document of exynos-ppmu.c driver
- Drop the devfreq-event type patch because it isnt' clear
- Fix bug of devfreq_event_add_edev() when fail to execute device_register()
- Modify resource free operation on following functions:
  : devfreq_event_remove_edev()
  : devfreq_event_release_edev()

Changes from v6:
(https://lkml.org/lkml/2014/12/28/139)
- This patchset is based on v3.19-rc3.
1. devfreq-event class driver
- Fix build break if devfreq-event framework is off
- Add resource-managed function for devfreq-event device
: devm_devfreq_event_add_edev()
: devm_devfreq_event_remove_edev()

Changes from v5:
(https://lkml.org/lkml/2014/12/22/527)
- Rebase these patch-set on v3.19-rc1 and Test it.
1. exynos-ppmu.c
- Change the error value when of_iomap() fail to map the memory
- Remove owner setting of platform_driver
- Add exynos_ppmu_disable() function
2. exynos dts file
- Add PPMU node to Exynos3250-based Monk board
- Remove ppmu_cpu node on Exynos4412-based TRATS2 board and add ppmu_leftbus/rightbus node

Changes from v4:
(https://lkml.org/lkml/2014/12/16/511)
1. devfreq-event class driver
- Add devfreq_event_get_edev_count() function
- Modify the simple description of devfreq-event framework in devfreq-event.c
- Minimize the usage range of global lock usage in devfreq_event_add_edev()
- Remove '_is_enabled()' function pointer in devfreq_event_ops structure
- Add separte CONFIG_PM_DEVFREQ_EVENT configuration
- Add new devfreq-event.h header file including devfreq-event helper functions
2. exynos dts file
- Add new patch to support PPMU with DEVFREQ-event on Exynos4412-based TRATS2

Changes from v3:
(https://lkml.org/lkml/2014/12/12/219)
1. devfreq-event class driver
- Fix return value of devfreq_event_get_event()
- Add new structure devfreq_event_data for devfreq_event_get_event()
- Modify the prototype of devfreq_event_get_event() function
- Call of_node_put after calling of_parse_phandle() to decrement refcount
2. exynos-ppmu driver
- Modify usage of devfreq_event_get_event() function
  according to new prototype of this funciton
- Add the additional description to exynos-ppmu.txt how to add PPMU node
  in board dts file
- Use 'PPMU_EVENT' macro to remove duplicate codes
- Add the support of PPMU for Exynos5260
3. exynos dts file
- Add missing PPMU_FSYS node to exynos3250.dtsi
- Fix 'ppmu_mfc_l' node name as 'ppmu_mfc' because exynos3250 has only one MFC IP.
- Add missing PPMU_ACP/G3D to exynos4.dtsi
4. etc
- Fix wrong abbreviation of PPMU (PPMU :Platform Performance Monitoring Unit)
- Add new patch to support the PPMU of Exynos5260 SoC

Changes from v2:
(https://lkml.org/lkml/2014/12/9/304)
1. devfreq-event class driver
- Rename all the helper functions of devfreq-event device
- Add devfreq_event_remove_edev() instead of devfreq_put_event_dev()
- Add devfreq_event_release_edev() to initialize it before put device
- Add the detailed description of devfreq-event API
- Add the attributes of devfreq-event class (enable_count)
- Check the overflow about event/total_event data in devfreq_event_get_event()
- Remove the 'exclusive flag' feature
- Set set_event()/get_event() functions as mandary
- Add missing of_node_put() call
- Change variable type of 'get_event()' funciton from 'int' to 'u64'
2. exynos-ppmu driver
- Remove un-used field (struct devfreq)
- Use 'of_get_child_by_name()' instead of 'of_find_node_by_name()'
- Add missing of_node_put() call
- Fix wrong clock control
- Use devfreq_event_remove_edev() instead of devfreq_remove_device()
- Add the documentation for exynos-ppmu driver
- Remove 'enable/disable/is_enabled/reset' function of exynos-ppmu driver
3. exynos3250-rinato.dts
- Add ppmu_{leftbus|rightbus} dt node and remove ppmu_cpu dt node

Changes from v1:
(https://lkml.org/lkml/2014/9/5/11)
- 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 (9):
  devfreq: event: Add new devfreq_event class to provide basic data for devfreq governor
  devfreq: event: Add resource-managed function for devfreq-event device
  devfreq: event: Add exynos-ppmu devfreq-event driver
  devfreq: event: Add documentation for exynos-ppmu devfreq-event driver
  ARM: dts: Add PPMU dt node for Exynos3250 SoC
  ARM: dts: Add PPMU dt node for Exynos4 SoCs
  ARM: dts: Add PPMU dt node for Exynos5260 SoC
  ARM: dts: exynos: Add PPMU node to Exynos3250-based Rinato/Monk board
  ARM: dts: exynos: Add PPMU node for Exynos4412-based TRATS2 board

 .../bindings/devfreq/event/exynos-ppmu.txt         | 110 +++++
 arch/arm/boot/dts/exynos3250-monk.dts              |  40 ++
 arch/arm/boot/dts/exynos3250-rinato.dts            |  40 ++
 arch/arm/boot/dts/exynos3250.dtsi                  |  74 +++
 arch/arm/boot/dts/exynos4.dtsi                     | 108 +++++
 arch/arm/boot/dts/exynos4210.dtsi                  |   8 +
 arch/arm/boot/dts/exynos4412-trats2.dts            |  40 ++
 arch/arm/boot/dts/exynos5260.dtsi                  |  90 ++++
 drivers/devfreq/Kconfig                            |   2 +
 drivers/devfreq/Makefile                           |   6 +-
 drivers/devfreq/devfreq-event.c                    | 511 +++++++++++++++++++++
 drivers/devfreq/event/Kconfig                      |  25 +
 drivers/devfreq/event/Makefile                     |   2 +
 drivers/devfreq/event/exynos-ppmu.c                | 398 ++++++++++++++++
 include/linux/devfreq-event.h                      | 186 ++++++++
 15 files changed, 1639 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/devfreq/event/exynos-ppmu.txt
 create mode 100644 drivers/devfreq/devfreq-event.c
 create mode 100644 drivers/devfreq/event/Kconfig
 create mode 100644 drivers/devfreq/event/Makefile
 create mode 100644 drivers/devfreq/event/exynos-ppmu.c
 create mode 100644 include/linux/devfreq-event.h

-- 
1.8.5.5


^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-01-20  7:25 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-01-20  6:59 Re: [PATCHv8 1/9] devfreq: event: Add new devfreq_event class to provide basic data for devfreq governor MyungJoo Ham
2015-01-20  7:25 ` Chanwoo Choi
  -- strict thread matches above, loose matches on Subject: below --
2015-01-20  4:34 MyungJoo Ham
2015-01-20  5:29 ` Chanwoo Choi
2015-01-12 12:34 [PATCHv8 0/9] devfreq: Add devfreq-event class to provide raw data for devfreq device Chanwoo Choi
2015-01-12 12:34 ` [PATCHv8 1/9] devfreq: event: Add new devfreq_event class to provide basic data for devfreq governor Chanwoo Choi
2015-01-19 10:12   ` Chanwoo Choi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox