* Re: [PATCH v8 0/5] DEVFREQ, DVFS Framework for Non-CPU Devices.
From: Turquette, Mike @ 2011-08-29 19:22 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Len Brown, Greg Kroah-Hartman, Kyungmin Park, MyungJoo Ham,
linux-pm, Thomas Gleixner
In-Reply-To: <201108272235.39334.rjw@sisk.pl>
Rafael,
Locking doesn't seem safe in v8; I replied with my comments.
I should be free this week to review the next round a little more
quickly. Last week was hectic.
Regards,
Mike
2011/8/27 Rafael J. Wysocki <rjw@sisk.pl>:
> Mike, are patches [3-5/5] in this revision fine by you?
>
> Rafael
>
>
> On Wednesday, August 24, 2011, MyungJoo Ham wrote:
>> The patchset revision v8 has minor updates since v7 and v6.
>> - Allow governors to have their own sysfs interface and init/exit callbacks.
>>
>> The patches 1/5 (OPP notifier) and 2/5 (DEVFREQ core) have no changes since v7.
>> There has been reordering between "add common sysfs interfaces" patch
>> and "add basic governors" (3/5 and 5/5)
>> "add internal interfaces for governors (4/5)" patch has been newly
>> introduced at v8 patchset.
>>
>> For a usage example, please look at
>> http://git.infradead.org/users/kmpark/linux-2.6-samsung/shortlog/refs/heads/devfreq
>>
>> In the above git tree, DVFS (dynamic voltage and frequency scaling) mechanism
>> is applied to the memory bus of Exynos4210 for Exynos4210-NURI boards.
>> In the example, the LPDDR2 DRAM frequency changes between 133, 266, and 400MHz
>> and other related clocks simply follow the determined DDR RAM clock.
>>
>> The DEVFREQ driver for Exynos4210 memory bus is at
>> /drivers/devfreq/exynos4210_memorybus.c in the git tree.
>>
>> In the dd (writing and reading 360MiB) test with NURI board, the memory
>> throughput was not changed (the performance is not deteriorated) while
>> the SoC power consumption has been reduced by 1%. When the memory access
>> is not that intense while the CPU is heavily used, the SoC power consumption
>> has been reduced by 6%. The power consumption has been compared with the
>> case using the conventional Exynos4210 CPUFREQ driver, which sets memory
>> bus frequency according to the CPU core frequency. Besides, when the CPU core
>> running slow and the memory access is intense, the performance (memory
>> throughput) has been increased by 11% (with higher SoC power consumption of
>> 5%). The tested governor is "simple-ondemand".
>>
>> MyungJoo Ham (5):
>> PM / OPP: Add OPP availability change notifier.
>> PM: Introduce DEVFREQ: generic DVFS framework with device-specific
>> OPPs
>> PM / DEVFREQ: add common sysfs interfaces
>> PM / DEVFREQ: add internal interfaces for governors
>> PM / DEVFREQ: add basic governors
>>
>> Documentation/ABI/testing/sysfs-devices-power | 46 +++
>> drivers/Kconfig | 2 +
>> drivers/Makefile | 2 +
>> drivers/base/power/opp.c | 29 ++
>> drivers/devfreq/Kconfig | 75 ++++
>> drivers/devfreq/Makefile | 5 +
>> drivers/devfreq/devfreq.c | 463 +++++++++++++++++++++++++
>> drivers/devfreq/governor.h | 20 +
>> drivers/devfreq/governor_performance.c | 24 ++
>> drivers/devfreq/governor_powersave.c | 24 ++
>> drivers/devfreq/governor_simpleondemand.c | 88 +++++
>> drivers/devfreq/governor_userspace.c | 119 +++++++
>> include/linux/devfreq.h | 150 ++++++++
>> include/linux/opp.h | 12 +
>> 14 files changed, 1059 insertions(+), 0 deletions(-)
>> create mode 100644 drivers/devfreq/Kconfig
>> create mode 100644 drivers/devfreq/Makefile
>> create mode 100644 drivers/devfreq/devfreq.c
>> create mode 100644 drivers/devfreq/governor.h
>> create mode 100644 drivers/devfreq/governor_performance.c
>> create mode 100644 drivers/devfreq/governor_powersave.c
>> create mode 100644 drivers/devfreq/governor_simpleondemand.c
>> create mode 100644 drivers/devfreq/governor_userspace.c
>> create mode 100644 include/linux/devfreq.h
>>
>>
>
>
^ permalink raw reply
* Re: [PATCH v8 4/5] PM / DEVFREQ: add internal interfaces for governors
From: Turquette, Mike @ 2011-08-29 19:21 UTC (permalink / raw)
To: MyungJoo Ham
Cc: Len Brown, Greg Kroah-Hartman, Kyungmin Park, linux-pm,
Thomas Gleixner
In-Reply-To: <1314174131-14194-5-git-send-email-myungjoo.ham@samsung.com>
On Wed, Aug 24, 2011 at 1:22 AM, MyungJoo Ham <myungjoo.ham@samsung.com> wrote:
> - get_devfreq(): governors may convert struct dev -> struct devfreq
> - update_devfreq(): governors may notify DEVFREQ core to reevaluate
> frequencies.
> - Governors may have .init and .exit callbacks
>
> In order to use the internal interface, get_devfreq() and
> update_devfreq(), governor should include "governor.h"
>
> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Please see patch 3 and 5 for my thoughts on locking in struct devfreq.
Also, this patch doesn't need to be separate, but can be rolled into
patch 3.
Regards,
Mike
> ---
> drivers/devfreq/devfreq.c | 32 ++++++++++++++++++++++++--------
> drivers/devfreq/governor.h | 20 ++++++++++++++++++++
> include/linux/devfreq.h | 2 ++
> 3 files changed, 46 insertions(+), 8 deletions(-)
> create mode 100644 drivers/devfreq/governor.h
>
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 5bbece6..8de3b21 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -65,10 +65,10 @@ static struct devfreq *find_device_devfreq(struct device *dev)
>
> /**
> * get_devfreq() - find devfreq struct. a wrapped find_device_devfreq()
> - * with mutex protection.
> + * with mutex protection. exported for governors
> * @dev: device pointer used to lookup device devfreq.
> */
> -static struct devfreq *get_devfreq(struct device *dev)
> +struct devfreq *get_devfreq(struct device *dev)
> {
> struct devfreq *ret;
>
> @@ -113,17 +113,15 @@ static int devfreq_do(struct devfreq *devfreq)
> }
>
> /**
> - * devfreq_update() - Notify that the device OPP has been changed.
> - * @dev: the device whose OPP has been changed.
> + * update_devfreq() - Notify that the device OPP or frequency requirement
> + * has been changed. This function is exported for governors.
> + * @devfreq: the devfreq instance.
> */
> -static int devfreq_update(struct notifier_block *nb, unsigned long type,
> - void *devp)
> +int update_devfreq(struct devfreq *devfreq)
> {
> - struct devfreq *devfreq;
> int err = 0;
>
> mutex_lock(&devfreq_list_lock);
> - devfreq = container_of(nb, struct devfreq, nb);
> /* Reevaluate the proper frequency */
> err = devfreq_do(devfreq);
> mutex_unlock(&devfreq_list_lock);
> @@ -131,6 +129,18 @@ static int devfreq_update(struct notifier_block *nb, unsigned long type,
> }
>
> /**
> + * devfreq_update() - Notify that the device OPP has been changed.
> + * @dev: the device whose OPP has been changed.
> + */
> +static int devfreq_update(struct notifier_block *nb, unsigned long type,
> + void *devp)
> +{
> + struct devfreq *devfreq = container_of(nb, struct devfreq, nb);
> +
> + return update_devfreq(devfreq);
> +}
> +
> +/**
> * devfreq_monitor() - Periodically run devfreq_do()
> * @work: the work struct used to run devfreq_monitor periodically.
> *
> @@ -254,6 +264,9 @@ int devfreq_add_device(struct device *dev, struct devfreq_dev_profile *profile,
>
> list_add(&devfreq->node, &devfreq_list);
>
> + if (governor->init)
> + governor->init(devfreq);
> +
> if (devfreq_wq && devfreq->next_polling && !polling) {
> polling = true;
> queue_delayed_work(devfreq_wq, &devfreq_work,
> @@ -295,6 +308,9 @@ int devfreq_remove_device(struct device *dev)
>
> sysfs_unmerge_group(&dev->kobj, &dev_attr_group);
>
> + if (devfreq->governor->exit)
> + devfreq->governor->exit(devfreq);
> +
> list_del(&devfreq->node);
> srcu_notifier_chain_unregister(nh, &devfreq->nb);
> kfree(devfreq);
> diff --git a/drivers/devfreq/governor.h b/drivers/devfreq/governor.h
> new file mode 100644
> index 0000000..bb5d964
> --- /dev/null
> +++ b/drivers/devfreq/governor.h
> @@ -0,0 +1,20 @@
> +/*
> + * governor.h - internal header for governors.
> + *
> + * Copyright (C) 2011 Samsung Electronics
> + * MyungJoo Ham <myungjoo.ham@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 header is for devfreq governors in drivers/devfreq/
> + */
> +
> +#ifndef _GOVERNOR_H
> +#define _GOVERNOR_H
> +
> +extern struct devfreq *get_devfreq(struct device *dev);
> +extern int update_devfreq(struct devfreq *devfreq);
> +
> +#endif /* _GOVERNOR_H */
> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
> index 8252238..fdc6916 100644
> --- a/include/linux/devfreq.h
> +++ b/include/linux/devfreq.h
> @@ -46,6 +46,8 @@ struct devfreq_dev_profile {
> struct devfreq_governor {
> char name[DEVFREQ_NAME_LEN];
> int (*get_target_freq)(struct devfreq *this, unsigned long *freq);
> + int (*init)(struct devfreq *this);
> + void (*exit)(struct devfreq *this);
> };
>
> /**
> --
> 1.7.4.1
>
>
^ permalink raw reply
* Re: [PATCH v8 3/5] PM / DEVFREQ: add common sysfs interfaces
From: Turquette, Mike @ 2011-08-29 19:17 UTC (permalink / raw)
To: MyungJoo Ham
Cc: Len Brown, Greg Kroah-Hartman, Kyungmin Park, linux-pm,
Thomas Gleixner
In-Reply-To: <1314174131-14194-4-git-send-email-myungjoo.ham@samsung.com>
On Wed, Aug 24, 2011 at 1:22 AM, MyungJoo Ham <myungjoo.ham@samsung.com> wrote:
> Device specific sysfs interface /sys/devices/.../power/devfreq_*
> - governor R: name of governor
> - cur_freq R: current frequency
> - max_freq R: maximum operable frequency
> - min_freq R: minimum operable frequency
> - polling_interval R: polling interval in ms given with devfreq profile
> W: update polling interval.
>
> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>
> --
> Changed from v7
> - removed set_freq from the common devfreq interface
> - added get_devfreq, a mutex-protected wrapper for find_device_devfreq
> (for sysfs interfaces and later with governor-support)
> - corrected ABI documentation.
>
> Changed from v6
> - poling_interval is writable.
>
> Changed from v5
> - updated devferq_update usage.
>
> Changed from v4
> - removed system-wide sysfs interface
> - removed tickling sysfs interface
> - added set_freq for userspace governor (and any other governors that
> require user input)
>
> Changed from v3
> - corrected sysfs API usage
> - corrected error messages
> - moved sysfs entry location
> - added sysfs entries
>
> Changed from v2
> - add ABI entries for devfreq sysfs interface
> ---
> Documentation/ABI/testing/sysfs-devices-power | 37 ++++++
> drivers/devfreq/devfreq.c | 150 +++++++++++++++++++++++++
> 2 files changed, 187 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-devices-power b/Documentation/ABI/testing/sysfs-devices-power
> index 8ffbc25..57f4591 100644
> --- a/Documentation/ABI/testing/sysfs-devices-power
> +++ b/Documentation/ABI/testing/sysfs-devices-power
> @@ -165,3 +165,40 @@ Description:
>
> Not all drivers support this attribute. If it isn't supported,
> attempts to read or write it will yield I/O errors.
> +
> +What: /sys/devices/.../power/devfreq_governor
> +Date: July 2011
> +Contact: MyungJoo Ham <myungjoo.ham@samsung.com>
> +Description:
> + The /sys/devices/.../power/devfreq_governor shows the name
> + of the governor used by the corresponding device.
> +
> +What: /sys/devices/.../power/devfreq_cur_freq
> +Date: July 2011
> +Contact: MyungJoo Ham <myungjoo.ham@samsung.com>
> +Description:
> + The /sys/devices/.../power/devfreq_cur_freq shows the current
> + frequency of the corresponding device.
> +
> +What: /sys/devices/.../power/devfreq_max_freq
> +Date: July 2011
> +Contact: MyungJoo Ham <myungjoo.ham@samsung.com>
> +Description:
> + The /sys/devices/.../power/devfreq_max_freq shows the
> + maximum operable frequency of the corresponding device.
> +
> +What: /sys/devices/.../power/devfreq_min_freq
> +Date: July 2011
> +Contact: MyungJoo Ham <myungjoo.ham@samsung.com>
> +Description:
> + The /sys/devices/.../power/devfreq_min_freq shows the
> + minimum operable frequency of the corresponding device.
> +
> +What: /sys/devices/.../power/devfreq_polling_interval
> +Date: July 2011
> +Contact: MyungJoo Ham <myungjoo.ham@samsung.com>
> +Description:
> + The /sys/devices/.../power/devfreq_polling_interval sets and
> + shows the requested polling interval of the corresponding
> + device. The values are represented in ms. If the value is less
> + than 1 jiffy, it is considered to be 0, which means no polling.
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index df63bdc..5bbece6 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -37,6 +37,8 @@ static struct delayed_work devfreq_work;
> static LIST_HEAD(devfreq_list);
> static DEFINE_MUTEX(devfreq_list_lock);
>
> +static struct attribute_group dev_attr_group;
> +
> /**
> * find_device_devfreq() - find devfreq struct using device pointer
> * @dev: device pointer used to lookup device devfreq.
> @@ -62,6 +64,22 @@ static struct devfreq *find_device_devfreq(struct device *dev)
> }
>
> /**
> + * get_devfreq() - find devfreq struct. a wrapped find_device_devfreq()
> + * with mutex protection.
> + * @dev: device pointer used to lookup device devfreq.
> + */
> +static struct devfreq *get_devfreq(struct device *dev)
> +{
> + struct devfreq *ret;
> +
> + mutex_lock(&devfreq_list_lock);
> + ret = find_device_devfreq(dev);
> + mutex_unlock(&devfreq_list_lock);
> +
> + return ret;
> +}
Sorry I'll have to rescind my ACK. I just reviewed patch 5/5 and the
show_freq and restore_freq functions for the userspace governor do not
protect the private data accesses with a mutex. Looking a bit further
I see that they do call get_devfreq; I think the semantics for this
are wrong.
get_devfreq only protects the list as long as it takes to do a lookup.
However neither struct devfreq or struct userspace_data have a mutex
associated with them, so concurrent operations are not protected.
One heavy-handed way of solving this is to not have get_devfreq
release the mutex, and then have those functions call a new
put_devfreq which does release the mutex. However that is really bad
locking.
What do you think about putting a mutex in struct devfreq? The rule
for governors can be that access to the governor-specific private data
requires taking that devfreq's mutex.
Regards,
Mike
> +
> +/**
> * devfreq_do() - Check the usage profile of a given device and configure
> * frequency and voltage accordingly
> * @devfreq: devfreq info of the given device
> @@ -149,6 +167,8 @@ static void devfreq_monitor(struct work_struct *work)
> dev_err(devfreq->dev, "Due to devfreq_do error(%d), devfreq(%s) is removed from the device\n",
> error, devfreq->governor->name);
>
> + sysfs_unmerge_group(&devfreq->dev->kobj,
> + &dev_attr_group);
> list_del(&devfreq->node);
> kfree(devfreq);
>
> @@ -239,6 +259,8 @@ int devfreq_add_device(struct device *dev, struct devfreq_dev_profile *profile,
> queue_delayed_work(devfreq_wq, &devfreq_work,
> devfreq->next_polling);
> }
> +
> + sysfs_merge_group(&dev->kobj, &dev_attr_group);
> out:
> mutex_unlock(&devfreq_list_lock);
>
> @@ -271,6 +293,8 @@ int devfreq_remove_device(struct device *dev)
> goto out;
> }
>
> + sysfs_unmerge_group(&dev->kobj, &dev_attr_group);
> +
> list_del(&devfreq->node);
> srcu_notifier_chain_unregister(nh, &devfreq->nb);
> kfree(devfreq);
> @@ -279,6 +303,132 @@ out:
> return 0;
> }
>
> +static ssize_t show_governor(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct devfreq *df = get_devfreq(dev);
> +
> + if (IS_ERR(df))
> + return PTR_ERR(df);
> + if (!df->governor)
> + return -EINVAL;
> +
> + return sprintf(buf, "%s\n", df->governor->name);
> +}
> +
> +static ssize_t show_freq(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct devfreq *df = get_devfreq(dev);
> +
> + if (IS_ERR(df))
> + return PTR_ERR(df);
> +
> + return sprintf(buf, "%lu\n", df->previous_freq);
> +}
> +
> +static ssize_t show_max_freq(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct devfreq *df = get_devfreq(dev);
> + unsigned long freq = ULONG_MAX;
> + struct opp *opp;
> +
> + if (IS_ERR(df))
> + return PTR_ERR(df);
> + if (!df->dev)
> + return -EINVAL;
> +
> + opp = opp_find_freq_floor(df->dev, &freq);
> + if (IS_ERR(opp))
> + return PTR_ERR(opp);
> +
> + return sprintf(buf, "%lu\n", freq);
> +}
> +
> +static ssize_t show_min_freq(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct devfreq *df = get_devfreq(dev);
> + unsigned long freq = 0;
> + struct opp *opp;
> +
> + if (IS_ERR(df))
> + return PTR_ERR(df);
> + if (!df->dev)
> + return -EINVAL;
> +
> + opp = opp_find_freq_ceil(df->dev, &freq);
> + if (IS_ERR(opp))
> + return PTR_ERR(opp);
> +
> + return sprintf(buf, "%lu\n", freq);
> +}
> +
> +static ssize_t show_polling_interval(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct devfreq *df = get_devfreq(dev);
> +
> + if (IS_ERR(df))
> + return PTR_ERR(df);
> + if (!df->profile)
> + return -EINVAL;
> +
> + return sprintf(buf, "%d\n", df->profile->polling_ms);
> +}
> +
> +static ssize_t store_polling_interval(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct devfreq *df = get_devfreq(dev);
> + unsigned int value;
> + int ret;
> +
> + if (IS_ERR(df))
> + return PTR_ERR(df);
> + if (!df->profile)
> + return -EINVAL;
> +
> + ret = sscanf(buf, "%u", &value);
> + if (ret != 1)
> + return -EINVAL;
> +
> + df->profile->polling_ms = value;
> + df->next_polling = df->polling_jiffies
> + = msecs_to_jiffies(value);
> +
> + mutex_lock(&devfreq_list_lock);
> + if (df->next_polling > 0 && !polling) {
> + polling = true;
> + queue_delayed_work(devfreq_wq, &devfreq_work,
> + df->next_polling);
> + }
> + mutex_unlock(&devfreq_list_lock);
> +
> + return count;
> +}
> +
> +static DEVICE_ATTR(devfreq_governor, 0444, show_governor, NULL);
> +static DEVICE_ATTR(devfreq_cur_freq, 0444, show_freq, NULL);
> +static DEVICE_ATTR(devfreq_max_freq, 0444, show_max_freq, NULL);
> +static DEVICE_ATTR(devfreq_min_freq, 0444, show_min_freq, NULL);
> +static DEVICE_ATTR(devfreq_polling_interval, 0644, show_polling_interval,
> + store_polling_interval);
> +static struct attribute *dev_entries[] = {
> + &dev_attr_devfreq_governor.attr,
> + &dev_attr_devfreq_cur_freq.attr,
> + &dev_attr_devfreq_max_freq.attr,
> + &dev_attr_devfreq_min_freq.attr,
> + &dev_attr_devfreq_polling_interval.attr,
> + NULL,
> +};
> +static struct attribute_group dev_attr_group = {
> + .name = power_group_name,
> + .attrs = dev_entries,
> +};
> +
> /**
> * devfreq_init() - Initialize data structure for devfreq framework and
> * start polling registered devfreq devices.
> --
> 1.7.4.1
>
>
^ permalink raw reply
* Re: [PATCH v8 5/5] PM / DEVFREQ: add basic governors
From: Turquette, Mike @ 2011-08-29 18:58 UTC (permalink / raw)
To: MyungJoo Ham
Cc: Len Brown, Greg Kroah-Hartman, Kyungmin Park, linux-pm,
Thomas Gleixner
In-Reply-To: <1314174131-14194-6-git-send-email-myungjoo.ham@samsung.com>
On Wed, Aug 24, 2011 at 1:22 AM, MyungJoo Ham <myungjoo.ham@samsung.com> wrote:
> Four CPUFREQ-like governors are provided as examples.
>
> powersave: use the lowest frequency possible. The user (device) should
> set the polling_ms as 0 because polling is useless for this governor.
>
> performance: use the highest freqeuncy possible. The user (device)
> should set the polling_ms as 0 because polling is useless for this
> governor.
>
> userspace: use the user specified frequency stored at
> devfreq.user_set_freq. With sysfs support in the following patch, a user
> may set the value with the sysfs interface.
>
> simple_ondemand: simplified version of CPUFREQ's ONDEMAND governor.
>
> When a user updates OPP entries (enable/disable/add), OPP framework
> automatically notifies DEVFREQ to update operating frequency
> accordingly. Thus, DEVFREQ users (device drivers) do not need to update
> DEVFREQ manually with OPP entry updates or set polling_ms for powersave
> , performance, userspace, or any other "static" governors.
>
> Note that these are given only as basic examples for governors and any
> devices with DEVFREQ may implement their own governors with the drivers
> and use them.
>
> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>
> ---
> Changed from v7
> - Userspace uses its own sysfs interface.
>
> Changed from v5
> - Seperated governor files from devfreq.c
> - Allow simple ondemand to be tuned for each device
> ---
> Documentation/ABI/testing/sysfs-devices-power | 9 ++
> drivers/devfreq/Kconfig | 36 ++++++++
> drivers/devfreq/Makefile | 4 +
> drivers/devfreq/governor_performance.c | 24 +++++
> drivers/devfreq/governor_powersave.c | 24 +++++
> drivers/devfreq/governor_simpleondemand.c | 88 ++++++++++++++++++
> drivers/devfreq/governor_userspace.c | 119 +++++++++++++++++++++++++
> include/linux/devfreq.h | 41 +++++++++
> 8 files changed, 345 insertions(+), 0 deletions(-)
> create mode 100644 drivers/devfreq/governor_performance.c
> create mode 100644 drivers/devfreq/governor_powersave.c
> create mode 100644 drivers/devfreq/governor_simpleondemand.c
> create mode 100644 drivers/devfreq/governor_userspace.c
>
> diff --git a/Documentation/ABI/testing/sysfs-devices-power b/Documentation/ABI/testing/sysfs-devices-power
> index 57f4591..c7f6977 100644
> --- a/Documentation/ABI/testing/sysfs-devices-power
> +++ b/Documentation/ABI/testing/sysfs-devices-power
> @@ -202,3 +202,12 @@ Description:
> shows the requested polling interval of the corresponding
> device. The values are represented in ms. If the value is less
> than 1 jiffy, it is considered to be 0, which means no polling.
> +
> +What: /sys/devices/.../power/devfreq_userspace_set_freq
How about just .../devfreq_set_freq? I think the userspace bit is
implied and the name is very long.
> +Date: August 2011
> +Contact: MyungJoo Ham <myungjoo.ham@samsung.com>
> +Description:
> + The /sys/devices/.../power/devfreq_userspace_set_freq sets
> + and shows the user specified frequency in kHz. This sysfs
> + entry is created and managed by userspace DEVFREQ governor.
> + If other governors are used, it won't be supported.
> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
> index 1fb42de..643b055 100644
> --- a/drivers/devfreq/Kconfig
> +++ b/drivers/devfreq/Kconfig
> @@ -34,6 +34,42 @@ menuconfig PM_DEVFREQ
>
> if PM_DEVFREQ
>
> +comment "DEVFREQ Governors"
> +
> +config DEVFREQ_GOV_SIMPLE_ONDEMAND
> + bool "Simple Ondemand"
> + help
> + Chooses frequency based on the recent load on the device. Works
> + similar as ONDEMAND governor of CPUFREQ does. A device with
> + Simple-Ondemand should be able to provide busy/total counter
> + values that imply the usage rate. A device may provide tuned
> + values to the governor with data field at devfreq_add_device().
> +
> +config DEVFREQ_GOV_PERFORMANCE
> + bool "Performance"
> + help
> + Sets the frequency at the maximum available frequency.
> + This governor always returns UINT_MAX as frequency so that
> + the DEVFREQ framework returns the highest frequency available
> + at any time.
> +
> +config DEVFREQ_GOV_POWERSAVE
> + bool "Powersave"
> + help
> + Sets the frequency at the minimum available frequency.
> + This governor always returns 0 as frequency so that
> + the DEVFREQ framework returns the lowest frequency available
> + at any time.
> +
> +config DEVFREQ_GOV_USERSPACE
> + bool "Userspace"
> + help
> + Sets the frequency at the user specified one.
> + This governor returns the user configured frequency if there
> + has been an input to /sys/devices/.../power/devfreq_set_freq.
> + Otherwise, the governor does not change the frequnecy
> + given at the initialization.
> +
> comment "DEVFREQ Drivers"
>
> endif # PM_DEVFREQ
> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
> index 168934a..4564a89 100644
> --- a/drivers/devfreq/Makefile
> +++ b/drivers/devfreq/Makefile
> @@ -1 +1,5 @@
> obj-$(CONFIG_PM_DEVFREQ) += devfreq.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
> +obj-$(CONFIG_DEVFREQ_GOV_USERSPACE) += governor_userspace.o
> diff --git a/drivers/devfreq/governor_performance.c b/drivers/devfreq/governor_performance.c
> new file mode 100644
> index 0000000..c47eff8
> --- /dev/null
> +++ b/drivers/devfreq/governor_performance.c
> @@ -0,0 +1,24 @@
> +/*
> + * linux/drivers/devfreq/governor_performance.c
> + *
> + * Copyright (C) 2011 Samsung Electronics
> + * MyungJoo Ham <myungjoo.ham@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.
> + */
> +
> +#include <linux/devfreq.h>
> +
> +static int devfreq_performance_func(struct devfreq *df,
> + unsigned long *freq)
> +{
> + *freq = UINT_MAX; /* devfreq_do will run "floor" */
> + return 0;
> +}
> +
> +struct devfreq_governor devfreq_performance = {
> + .name = "performance",
> + .get_target_freq = devfreq_performance_func,
> +};
> diff --git a/drivers/devfreq/governor_powersave.c b/drivers/devfreq/governor_powersave.c
> new file mode 100644
> index 0000000..4f128d8
> --- /dev/null
> +++ b/drivers/devfreq/governor_powersave.c
> @@ -0,0 +1,24 @@
> +/*
> + * linux/drivers/devfreq/governor_powersave.c
> + *
> + * Copyright (C) 2011 Samsung Electronics
> + * MyungJoo Ham <myungjoo.ham@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.
> + */
> +
> +#include <linux/devfreq.h>
> +
> +static int devfreq_powersave_func(struct devfreq *df,
> + unsigned long *freq)
> +{
> + *freq = 0; /* devfreq_do will run "ceiling" to 0 */
> + return 0;
> +}
> +
> +struct devfreq_governor devfreq_powersave = {
> + .name = "powersave",
> + .get_target_freq = devfreq_powersave_func,
> +};
> diff --git a/drivers/devfreq/governor_simpleondemand.c b/drivers/devfreq/governor_simpleondemand.c
> new file mode 100644
> index 0000000..18fe8be
> --- /dev/null
> +++ b/drivers/devfreq/governor_simpleondemand.c
> @@ -0,0 +1,88 @@
> +/*
> + * linux/drivers/devfreq/governor_simpleondemand.c
> + *
> + * Copyright (C) 2011 Samsung Electronics
> + * MyungJoo Ham <myungjoo.ham@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.
> + */
> +
> +#include <linux/errno.h>
> +#include <linux/devfreq.h>
> +#include <linux/math64.h>
> +
> +/* Default constants for DevFreq-Simple-Ondemand (DFSO) */
> +#define DFSO_UPTHRESHOLD (90)
> +#define DFSO_DOWNDIFFERENCTIAL (5)
> +static int devfreq_simple_ondemand_func(struct devfreq *df,
> + unsigned long *freq)
> +{
> + struct devfreq_dev_status stat;
> + int err = df->profile->get_dev_status(df->dev, &stat);
> + unsigned long long a, b;
> + unsigned int dfso_upthreshold = DFSO_UPTHRESHOLD;
> + unsigned int dfso_downdifferential = DFSO_DOWNDIFFERENCTIAL;
> + struct devfreq_simple_ondemand_data *data = df->data;
> +
> + if (err)
> + return err;
> +
> + if (data) {
> + if (data->upthreshold)
> + dfso_upthreshold = data->upthreshold;
> + if (data->downdifferential)
> + dfso_downdifferential = data->downdifferential;
> + }
> + if (dfso_upthreshold > 100 ||
> + dfso_upthreshold < dfso_downdifferential)
> + return -EINVAL;
> +
> + /* Assume MAX if it is going to be divided by zero */
> + if (stat.total_time == 0) {
> + *freq = UINT_MAX;
> + return 0;
> + }
> +
> + /* Prevent overflow */
> + if (stat.busy_time >= (1 << 24) || stat.total_time >= (1 << 24)) {
> + stat.busy_time >>= 7;
> + stat.total_time >>= 7;
> + }
> +
> + /* Set MAX if it's busy enough */
> + if (stat.busy_time * 100 >
> + stat.total_time * dfso_upthreshold) {
> + *freq = UINT_MAX;
> + return 0;
> + }
> +
> + /* Set MAX if we do not know the initial frequency */
> + if (stat.current_frequency == 0) {
> + *freq = UINT_MAX;
> + return 0;
> + }
> +
> + /* Keep the current frequency */
> + if (stat.busy_time * 100 >
> + stat.total_time * (dfso_upthreshold - dfso_downdifferential)) {
> + *freq = stat.current_frequency;
> + return 0;
> + }
> +
> + /* Set the desired frequency based on the load */
> + a = stat.busy_time;
> + a *= stat.current_frequency;
> + b = div_u64(a, stat.total_time);
> + b *= 100;
> + b = div_u64(b, (dfso_upthreshold - dfso_downdifferential / 2));
> + *freq = (unsigned long) b;
> +
> + return 0;
> +}
> +
> +struct devfreq_governor devfreq_simple_ondemand = {
> + .name = "simple_ondemand",
> + .get_target_freq = devfreq_simple_ondemand_func,
> +};
> diff --git a/drivers/devfreq/governor_userspace.c b/drivers/devfreq/governor_userspace.c
> new file mode 100644
> index 0000000..53a4574
> --- /dev/null
> +++ b/drivers/devfreq/governor_userspace.c
> @@ -0,0 +1,119 @@
> +/*
> + * linux/drivers/devfreq/governor_simpleondemand.c
> + *
> + * Copyright (C) 2011 Samsung Electronics
> + * MyungJoo Ham <myungjoo.ham@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.
> + */
> +
> +#include <linux/slab.h>
> +#include <linux/device.h>
> +#include <linux/devfreq.h>
> +#include <linux/pm.h>
> +#include "governor.h"
> +
> +struct userspace_data {
> + unsigned long user_frequency;
> + bool valid;
> +};
> +
> +static int devfreq_userspace_func(struct devfreq *df, unsigned long *freq)
> +{
> + struct userspace_data *data = df->data;
> +
> + if (!data->valid)
> + *freq = df->previous_freq; /* No user freq specified yet */
> + else
> + *freq = data->user_frequency;
> + return 0;
> +}
> +
> +static ssize_t store_freq(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct devfreq *devfreq = get_devfreq(dev);
> + struct userspace_data *data;
> + unsigned long wanted;
> + int err = 0;
> +
> + if (IS_ERR(devfreq)) {
> + err = PTR_ERR(devfreq);
> + goto out;
> + }
> + data = devfreq->data;
> +
> + sscanf(buf, "%lu", &wanted);
> + data->user_frequency = wanted;
> + data->valid = true;
> + err = update_devfreq(devfreq);
> + if (err == 0)
> + err = count;
> +out:
> + return err;
> +}
> +
> +static ssize_t show_freq(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + struct devfreq *devfreq = get_devfreq(dev);
> + struct userspace_data *data;
> + int err = 0;
> +
> + if (IS_ERR(devfreq)) {
> + err = PTR_ERR(devfreq);
> + goto out;
> + }
> + data = devfreq->data;
> +
> + if (data->valid)
> + err = sprintf(buf, "%lu\n", data->user_frequency);
> + else
> + err = sprintf(buf, "undefined\n");
> +out:
> + return err;
> +}
Shouldn't accesses to devfreq->data be protected by a mutex?
Regards,
Mike
> +
> +static DEVICE_ATTR(devfreq_userspace_set_freq, 0644, show_freq, store_freq);
> +static struct attribute *dev_entries[] = {
> + &dev_attr_devfreq_userspace_set_freq.attr,
> + NULL,
> +};
> +static struct attribute_group dev_attr_group = {
> + .name = power_group_name,
> + .attrs = dev_entries,
> +};
> +
> +static int userspace_init(struct devfreq *devfreq)
> +{
> + int err = 0;
> + struct userspace_data *data = kzalloc(sizeof(struct userspace_data),
> + GFP_KERNEL);
> +
> + if (!data) {
> + err = -ENOMEM;
> + goto out;
> + }
> + data->valid = false;
> + devfreq->data = data;
> +
> + sysfs_merge_group(&devfreq->dev->kobj, &dev_attr_group);
> +out:
> + return err;
> +}
> +
> +static void userspace_exit(struct devfreq *devfreq)
> +{
> + sysfs_unmerge_group(&devfreq->dev->kobj, &dev_attr_group);
> + kfree(devfreq->data);
> + devfreq->data = NULL;
> +}
> +
> +struct devfreq_governor devfreq_userspace = {
> + .name = "userspace",
> + .get_target_freq = devfreq_userspace_func,
> + .init = userspace_init,
> + .exit = userspace_exit,
> +};
> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
> index fdc6916..cbafcdf 100644
> --- a/include/linux/devfreq.h
> +++ b/include/linux/devfreq.h
> @@ -13,6 +13,7 @@
> #ifndef __LINUX_DEVFREQ_H__
> #define __LINUX_DEVFREQ_H__
>
> +#include <linux/opp.h>
> #include <linux/notifier.h>
>
> #define DEVFREQ_NAME_LEN 16
> @@ -65,6 +66,8 @@ struct devfreq_governor {
> * "devfreq_monitor" executions to reevaluate
> * frequency/voltage of the device. Set by
> * profile's polling_ms interval.
> + * @user_set_freq User specified adequete frequency value (thru sysfs
> + * interface). Governors may and may not use this value.
> * @data Private data of the governor. The devfreq framework does not
> * touch this.
> *
> @@ -82,6 +85,7 @@ struct devfreq {
> unsigned long previous_freq;
> unsigned int next_polling;
>
> + unsigned long user_set_freq; /* governors may ignore this. */
> void *data; /* private data for governors */
> };
>
> @@ -91,6 +95,37 @@ extern int devfreq_add_device(struct device *dev,
> struct devfreq_governor *governor,
> void *data);
> extern int devfreq_remove_device(struct device *dev);
> +
> +#ifdef CONFIG_DEVFREQ_GOV_POWERSAVE
> +extern struct devfreq_governor devfreq_powersave;
> +#endif
> +#ifdef CONFIG_DEVFREQ_GOV_PERFORMANCE
> +extern struct devfreq_governor devfreq_performance;
> +#endif
> +#ifdef CONFIG_DEVFREQ_GOV_USERSPACE
> +extern struct devfreq_governor devfreq_userspace;
> +#endif
> +#ifdef CONFIG_DEVFREQ_GOV_SIMPLE_ONDEMAND
> +extern struct devfreq_governor devfreq_simple_ondemand;
> +/**
> + * struct devfreq_simple_ondemand_data - void *data fed to struct devfreq
> + * and devfreq_add_device
> + * @ upthreshold If the load is over this value, the frequency jumps.
> + * Specify 0 to use the default. Valid value = 0 to 100.
> + * @ downdifferential If the load is under upthreshold - downdifferential,
> + * the governor may consider slowing the frequency down.
> + * Specify 0 to use the default. Valid value = 0 to 100.
> + * downdifferential < upthreshold must hold.
> + *
> + * If the fed devfreq_simple_ondemand_data pointer is NULL to the governor,
> + * the governor uses the default values.
> + */
> +struct devfreq_simple_ondemand_data {
> + unsigned int upthreshold;
> + unsigned int downdifferential;
> +};
> +#endif
> +
> #else /* !CONFIG_PM_DEVFREQ */
> static int devfreq_add_device(struct device *dev,
> struct devfreq_dev_profile *profile,
> @@ -104,6 +139,12 @@ static int devfreq_remove_device(struct device *dev)
> {
> return 0;
> }
> +
> +#define devfreq_powersave NULL
> +#define devfreq_performance NULL
> +#define devfreq_userspace NULL
> +#define devfreq_simple_ondemand NULL
> +
> #endif /* CONFIG_PM_DEVFREQ */
>
> #endif /* __LINUX_DEVFREQ_H__ */
> --
> 1.7.4.1
>
>
^ permalink raw reply
* Re: [PATCH v8 3/5] PM / DEVFREQ: add common sysfs interfaces
From: Turquette, Mike @ 2011-08-29 18:49 UTC (permalink / raw)
To: MyungJoo Ham
Cc: Len Brown, Greg Kroah-Hartman, Kyungmin Park, linux-pm,
Thomas Gleixner
In-Reply-To: <1314174131-14194-4-git-send-email-myungjoo.ham@samsung.com>
On Wed, Aug 24, 2011 at 1:22 AM, MyungJoo Ham <myungjoo.ham@samsung.com> wrote:
> Device specific sysfs interface /sys/devices/.../power/devfreq_*
> - governor R: name of governor
> - cur_freq R: current frequency
> - max_freq R: maximum operable frequency
> - min_freq R: minimum operable frequency
> - polling_interval R: polling interval in ms given with devfreq profile
> W: update polling interval.
>
> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Looks good to me.
Reviewed-by: Mike Turquette <mturquette@ti.com>
Regards,
Mike
>
> --
> Changed from v7
> - removed set_freq from the common devfreq interface
> - added get_devfreq, a mutex-protected wrapper for find_device_devfreq
> (for sysfs interfaces and later with governor-support)
> - corrected ABI documentation.
>
> Changed from v6
> - poling_interval is writable.
>
> Changed from v5
> - updated devferq_update usage.
>
> Changed from v4
> - removed system-wide sysfs interface
> - removed tickling sysfs interface
> - added set_freq for userspace governor (and any other governors that
> require user input)
>
> Changed from v3
> - corrected sysfs API usage
> - corrected error messages
> - moved sysfs entry location
> - added sysfs entries
>
> Changed from v2
> - add ABI entries for devfreq sysfs interface
> ---
> Documentation/ABI/testing/sysfs-devices-power | 37 ++++++
> drivers/devfreq/devfreq.c | 150 +++++++++++++++++++++++++
> 2 files changed, 187 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-devices-power b/Documentation/ABI/testing/sysfs-devices-power
> index 8ffbc25..57f4591 100644
> --- a/Documentation/ABI/testing/sysfs-devices-power
> +++ b/Documentation/ABI/testing/sysfs-devices-power
> @@ -165,3 +165,40 @@ Description:
>
> Not all drivers support this attribute. If it isn't supported,
> attempts to read or write it will yield I/O errors.
> +
> +What: /sys/devices/.../power/devfreq_governor
> +Date: July 2011
> +Contact: MyungJoo Ham <myungjoo.ham@samsung.com>
> +Description:
> + The /sys/devices/.../power/devfreq_governor shows the name
> + of the governor used by the corresponding device.
> +
> +What: /sys/devices/.../power/devfreq_cur_freq
> +Date: July 2011
> +Contact: MyungJoo Ham <myungjoo.ham@samsung.com>
> +Description:
> + The /sys/devices/.../power/devfreq_cur_freq shows the current
> + frequency of the corresponding device.
> +
> +What: /sys/devices/.../power/devfreq_max_freq
> +Date: July 2011
> +Contact: MyungJoo Ham <myungjoo.ham@samsung.com>
> +Description:
> + The /sys/devices/.../power/devfreq_max_freq shows the
> + maximum operable frequency of the corresponding device.
> +
> +What: /sys/devices/.../power/devfreq_min_freq
> +Date: July 2011
> +Contact: MyungJoo Ham <myungjoo.ham@samsung.com>
> +Description:
> + The /sys/devices/.../power/devfreq_min_freq shows the
> + minimum operable frequency of the corresponding device.
> +
> +What: /sys/devices/.../power/devfreq_polling_interval
> +Date: July 2011
> +Contact: MyungJoo Ham <myungjoo.ham@samsung.com>
> +Description:
> + The /sys/devices/.../power/devfreq_polling_interval sets and
> + shows the requested polling interval of the corresponding
> + device. The values are represented in ms. If the value is less
> + than 1 jiffy, it is considered to be 0, which means no polling.
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index df63bdc..5bbece6 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -37,6 +37,8 @@ static struct delayed_work devfreq_work;
> static LIST_HEAD(devfreq_list);
> static DEFINE_MUTEX(devfreq_list_lock);
>
> +static struct attribute_group dev_attr_group;
> +
> /**
> * find_device_devfreq() - find devfreq struct using device pointer
> * @dev: device pointer used to lookup device devfreq.
> @@ -62,6 +64,22 @@ static struct devfreq *find_device_devfreq(struct device *dev)
> }
>
> /**
> + * get_devfreq() - find devfreq struct. a wrapped find_device_devfreq()
> + * with mutex protection.
> + * @dev: device pointer used to lookup device devfreq.
> + */
> +static struct devfreq *get_devfreq(struct device *dev)
> +{
> + struct devfreq *ret;
> +
> + mutex_lock(&devfreq_list_lock);
> + ret = find_device_devfreq(dev);
> + mutex_unlock(&devfreq_list_lock);
> +
> + return ret;
> +}
> +
> +/**
> * devfreq_do() - Check the usage profile of a given device and configure
> * frequency and voltage accordingly
> * @devfreq: devfreq info of the given device
> @@ -149,6 +167,8 @@ static void devfreq_monitor(struct work_struct *work)
> dev_err(devfreq->dev, "Due to devfreq_do error(%d), devfreq(%s) is removed from the device\n",
> error, devfreq->governor->name);
>
> + sysfs_unmerge_group(&devfreq->dev->kobj,
> + &dev_attr_group);
> list_del(&devfreq->node);
> kfree(devfreq);
>
> @@ -239,6 +259,8 @@ int devfreq_add_device(struct device *dev, struct devfreq_dev_profile *profile,
> queue_delayed_work(devfreq_wq, &devfreq_work,
> devfreq->next_polling);
> }
> +
> + sysfs_merge_group(&dev->kobj, &dev_attr_group);
> out:
> mutex_unlock(&devfreq_list_lock);
>
> @@ -271,6 +293,8 @@ int devfreq_remove_device(struct device *dev)
> goto out;
> }
>
> + sysfs_unmerge_group(&dev->kobj, &dev_attr_group);
> +
> list_del(&devfreq->node);
> srcu_notifier_chain_unregister(nh, &devfreq->nb);
> kfree(devfreq);
> @@ -279,6 +303,132 @@ out:
> return 0;
> }
>
> +static ssize_t show_governor(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct devfreq *df = get_devfreq(dev);
> +
> + if (IS_ERR(df))
> + return PTR_ERR(df);
> + if (!df->governor)
> + return -EINVAL;
> +
> + return sprintf(buf, "%s\n", df->governor->name);
> +}
> +
> +static ssize_t show_freq(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct devfreq *df = get_devfreq(dev);
> +
> + if (IS_ERR(df))
> + return PTR_ERR(df);
> +
> + return sprintf(buf, "%lu\n", df->previous_freq);
> +}
> +
> +static ssize_t show_max_freq(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct devfreq *df = get_devfreq(dev);
> + unsigned long freq = ULONG_MAX;
> + struct opp *opp;
> +
> + if (IS_ERR(df))
> + return PTR_ERR(df);
> + if (!df->dev)
> + return -EINVAL;
> +
> + opp = opp_find_freq_floor(df->dev, &freq);
> + if (IS_ERR(opp))
> + return PTR_ERR(opp);
> +
> + return sprintf(buf, "%lu\n", freq);
> +}
> +
> +static ssize_t show_min_freq(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct devfreq *df = get_devfreq(dev);
> + unsigned long freq = 0;
> + struct opp *opp;
> +
> + if (IS_ERR(df))
> + return PTR_ERR(df);
> + if (!df->dev)
> + return -EINVAL;
> +
> + opp = opp_find_freq_ceil(df->dev, &freq);
> + if (IS_ERR(opp))
> + return PTR_ERR(opp);
> +
> + return sprintf(buf, "%lu\n", freq);
> +}
> +
> +static ssize_t show_polling_interval(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct devfreq *df = get_devfreq(dev);
> +
> + if (IS_ERR(df))
> + return PTR_ERR(df);
> + if (!df->profile)
> + return -EINVAL;
> +
> + return sprintf(buf, "%d\n", df->profile->polling_ms);
> +}
> +
> +static ssize_t store_polling_interval(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct devfreq *df = get_devfreq(dev);
> + unsigned int value;
> + int ret;
> +
> + if (IS_ERR(df))
> + return PTR_ERR(df);
> + if (!df->profile)
> + return -EINVAL;
> +
> + ret = sscanf(buf, "%u", &value);
> + if (ret != 1)
> + return -EINVAL;
> +
> + df->profile->polling_ms = value;
> + df->next_polling = df->polling_jiffies
> + = msecs_to_jiffies(value);
> +
> + mutex_lock(&devfreq_list_lock);
> + if (df->next_polling > 0 && !polling) {
> + polling = true;
> + queue_delayed_work(devfreq_wq, &devfreq_work,
> + df->next_polling);
> + }
> + mutex_unlock(&devfreq_list_lock);
> +
> + return count;
> +}
> +
> +static DEVICE_ATTR(devfreq_governor, 0444, show_governor, NULL);
> +static DEVICE_ATTR(devfreq_cur_freq, 0444, show_freq, NULL);
> +static DEVICE_ATTR(devfreq_max_freq, 0444, show_max_freq, NULL);
> +static DEVICE_ATTR(devfreq_min_freq, 0444, show_min_freq, NULL);
> +static DEVICE_ATTR(devfreq_polling_interval, 0644, show_polling_interval,
> + store_polling_interval);
> +static struct attribute *dev_entries[] = {
> + &dev_attr_devfreq_governor.attr,
> + &dev_attr_devfreq_cur_freq.attr,
> + &dev_attr_devfreq_max_freq.attr,
> + &dev_attr_devfreq_min_freq.attr,
> + &dev_attr_devfreq_polling_interval.attr,
> + NULL,
> +};
> +static struct attribute_group dev_attr_group = {
> + .name = power_group_name,
> + .attrs = dev_entries,
> +};
> +
> /**
> * devfreq_init() - Initialize data structure for devfreq framework and
> * start polling registered devfreq devices.
> --
> 1.7.4.1
>
>
^ permalink raw reply
* Re: [PATCH pm-freezer 2/4] freezer: set PF_NOFREEZE on a dying task right before TASK_DEAD
From: Oleg Nesterov @ 2011-08-29 18:02 UTC (permalink / raw)
To: Tejun Heo; +Cc: linux-kernel, Paul Menage, containers, linux-pm
In-Reply-To: <20110829140509.GC18871@mtj.dyndns.org>
On 08/29, Tejun Heo wrote:
>
> There's no try_to_freeze() call in the exit path and the only
> necessary guarantee is that freezer doesn't hang waiting for zombies.
> Set PF_NOFREEZE right before setting tsk->state to TASK_DEAD instead.
Agreed.
But I'd like to repeat, this looks "asymmetrical". do_each_thread()
can't see the (auto)reaped tasks after they do exit_notify(). So we
can only see this PF_NOFREEZE if the thread becomes a zombie.
> @@ -1044,6 +1038,10 @@ NORET_TYPE void do_exit(long code)
>
> preempt_disable();
> exit_rcu();
> +
> + /* this task is now dead and freezer should ignore it */
> + current->flags |= PF_NOFREEZE;
> +
> /* causes final put_task_struct in finish_task_switch(). */
> tsk->state = TASK_DEAD;
May be freezing_slow_path() can check TASK_DEAD along with PF_NOFREEZE
instead? (or tsk->exit_state != 0 to avoid the asymmetry above). Just
to keep this logic in the freezer code. I dunno.
But this all is up to you and Rafael, I am not arguing. Just random
thoughts.
Oleg.
^ permalink raw reply
* Re: [PATCH pm-freezer 3/4] freezer: check freezing() before leaving FROZEN state
From: Oleg Nesterov @ 2011-08-29 17:21 UTC (permalink / raw)
To: Tejun Heo; +Cc: linux-kernel, Paul Menage, containers, linux-pm
In-Reply-To: <20110829171228.GA11339@redhat.com>
On 08/29, Oleg Nesterov wrote:
>
> Hmm. But I got lost a bit. Why do we need freezer_lock to set/clear
> PF_FROZEN ? OK, the code below takes freezer_lock for freezing().
> Is there any other reason?
Ah, at least we need it to serialize with __thaw_task() which does
"if (frozen) wake_up()".
Oleg.
^ permalink raw reply
* Re: [PATCH pm-freezer 3/4] freezer: check freezing() before leaving FROZEN state
From: Oleg Nesterov @ 2011-08-29 17:12 UTC (permalink / raw)
To: Tejun Heo; +Cc: linux-kernel, Paul Menage, containers, linux-pm
In-Reply-To: <20110829163533.GB9973@redhat.com>
On 08/29, Oleg Nesterov wrote:
>
> On 08/29, Tejun Heo wrote:
> >
> > --- work.orig/kernel/freezer.c
> > +++ work/kernel/freezer.c
> > @@ -60,6 +60,7 @@ bool __refrigerator(bool check_kthr_stop
> > */
> > spin_lock_irq(&freezer_lock);
> > current->flags |= PF_FROZEN;
> > +refreeze:
> > spin_unlock_irq(&freezer_lock);
> >
> > save = current->state;
> > @@ -78,8 +79,10 @@ bool __refrigerator(bool check_kthr_stop
> > schedule();
> > }
> >
> > - /* leave FROZEN */
> > + /* leave FROZEN after checking freezing() holding freezer_lock */
> > spin_lock_irq(&freezer_lock);
> > + if (freezing(current))
> > + goto refreeze;
>
> Looks like, you should move "save = current->state" up then.
Hmm. And afaics there is another problem. This can "livelock" if
check_kthr_stop && kthread_should_stop().
May be we should consolidate the freezer_lock's sections, something
like below?
Hmm. But I got lost a bit. Why do we need freezer_lock to set/clear
PF_FROZEN ? OK, the code below takes freezer_lock for freezing().
Is there any other reason?
Oleg.
bool __refrigerator(bool check_kthr_stop)
{
/* Hmm, should we be allowed to suspend when there are realtime
processes around? */
bool was_frozen = false;
long save;
save = current->state;
pr_debug("%s entered refrigerator\n", current->comm);
for (;;) {
set_current_state(TASK_UNINTERRUPTIBLE);
spin_lock_irq(&freezer_lock);
current->flags |= PF_FROZEN;
if (!freezing(current) ||
(check_kthr_stop && kthread_should_stop()))
current->flags &= ~PF_FROZEN;
spin_unlock_irq(&freezer_lock);
if (!current->flags & PF_FROZEN)
break;
was_frozen = true;
schedule();
}
spin_lock_irq(¤t->sighand->siglock);
recalc_sigpending(); /* We sent fake signal, clean it up */
spin_unlock_irq(¤t->sighand->siglock);
pr_debug("%s left refrigerator\n", current->comm);
/*
* Restore saved task state before returning. The mb'd version
* needs to be used; otherwise, it might silently break
* synchronization which depends on ordered task state change.
*/
set_current_state(save);
return was_frozen;
}
^ permalink raw reply
* Re: [PATCH pm-freezer 4/4] freezer: use lock_task_sighand() in fake_signal_wake_up()
From: Oleg Nesterov @ 2011-08-29 16:36 UTC (permalink / raw)
To: Tejun Heo; +Cc: linux-kernel, Paul Menage, containers, linux-pm
In-Reply-To: <20110829140621.GE18871@mtj.dyndns.org>
On 08/29, Tejun Heo wrote:
>
> --- work.orig/kernel/freezer.c
> +++ work/kernel/freezer.c
> @@ -103,9 +103,10 @@ static void fake_signal_wake_up(struct t
> {
> unsigned long flags;
>
> - spin_lock_irqsave(&p->sighand->siglock, flags);
> - signal_wake_up(p, 0);
> - spin_unlock_irqrestore(&p->sighand->siglock, flags);
> + if (lock_task_sighand(p, &flags)) {
> + signal_wake_up(p, 0);
> + unlock_task_sighand(p, &flags);
> + }
Yes, this looks correct.
Oleg.
^ permalink raw reply
* Re: [PATCH pm-freezer 3/4] freezer: check freezing() before leaving FROZEN state
From: Oleg Nesterov @ 2011-08-29 16:35 UTC (permalink / raw)
To: Tejun Heo; +Cc: linux-kernel, Paul Menage, containers, linux-pm
In-Reply-To: <20110829140549.GD18871@mtj.dyndns.org>
On 08/29, Tejun Heo wrote:
>
> --- work.orig/kernel/freezer.c
> +++ work/kernel/freezer.c
> @@ -60,6 +60,7 @@ bool __refrigerator(bool check_kthr_stop
> */
> spin_lock_irq(&freezer_lock);
> current->flags |= PF_FROZEN;
> +refreeze:
> spin_unlock_irq(&freezer_lock);
>
> save = current->state;
> @@ -78,8 +79,10 @@ bool __refrigerator(bool check_kthr_stop
> schedule();
> }
>
> - /* leave FROZEN */
> + /* leave FROZEN after checking freezing() holding freezer_lock */
> spin_lock_irq(&freezer_lock);
> + if (freezing(current))
> + goto refreeze;
Looks like, you should move "save = current->state" up then.
Oleg.
^ permalink raw reply
* Re: [PATCH pm-freezer 1/4] cgroup_freezer: fix freezer->state setting bug in freezer_change_state()
From: Oleg Nesterov @ 2011-08-29 16:00 UTC (permalink / raw)
To: Tejun Heo; +Cc: linux-kernel, Paul Menage, containers, linux-pm
In-Reply-To: <20110829140418.GB18871@mtj.dyndns.org>
On 08/29, Tejun Heo wrote:
>
> --- work.orig/kernel/cgroup_freezer.c
> +++ work/kernel/cgroup_freezer.c
> @@ -311,14 +311,14 @@ static int freezer_change_state(struct c
> if (goal_state == freezer->state)
> goto out;
>
> - freezer->state = goal_state;
> -
> switch (goal_state) {
> case CGROUP_THAWED:
> + freezer->state = CGROUP_THAWED;
> atomic_dec(&system_freezing_cnt);
> unfreeze_cgroup(cgroup, freezer);
> break;
> case CGROUP_FROZEN:
> + freezer->state = CGROUP_FREEZING;
At first glance, this is correct. I'll try to recheck.
But,
> atomic_inc(&system_freezing_cnt);
iiuc this becomes wrong... Suppose a user writes "FROZEN" twice,
before freezer->state becomes CGROUP_FROZEN.
I think we should actually fix the "goal_state == freezer->state"
check above.
Oleg.
^ permalink raw reply
* [PATCH v2 4/4] bq24022: Add support for the bq2407x family
From: Heiko Stübner @ 2011-08-29 15:59 UTC (permalink / raw)
To: Mark Brown, Liam Girdwood; +Cc: linux-pm, Philipp Zabel
In-Reply-To: <201108291755.15701.heiko@sntech.de>
The bq2407x family of charger ICs simply supports another machine
specific charging current above 500mA. To use this setting a
second gpio pin is used while the rest of the pins stay the same.
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
Changes since v1: use dev_err instead of dev_dbg in fatal places
drivers/regulator/Kconfig | 7 ++--
drivers/regulator/bq24022.c | 65 ++++++++++++++++++++++++++++++++++---
include/linux/regulator/bq24022.h | 7 ++++
3 files changed, 71 insertions(+), 8 deletions(-)
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index c7fd2c0..546a7a6 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -67,10 +67,11 @@ config REGULATOR_USERSPACE_CONSUMER
config REGULATOR_BQ24022
tristate "TI bq24022 Dual Input 1-Cell Li-Ion Charger IC"
help
- This driver controls a TI bq24022 Charger attached via
- GPIOs. The provided current regulator can enable/disable
+ This driver controls a TI bq24022 or bq2407x Charger attached
+ via GPIOs. The provided current regulator can enable/disable
charging select between 100 mA and 500 mA charging current
- limit.
+ limit. The bq2407x family also supports a machine specific
+ current limit above 500 mA.
config REGULATOR_MAX1586
tristate "Maxim 1586/1587 voltage regulator"
diff --git a/drivers/regulator/bq24022.c b/drivers/regulator/bq24022.c
index f75cd07..204ed58 100644
--- a/drivers/regulator/bq24022.c
+++ b/drivers/regulator/bq24022.c
@@ -19,15 +19,20 @@
#include <linux/gpio.h>
#include <linux/regulator/bq24022.h>
#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
struct bq24022 {
struct regulator_dev *rdev;
int gpio_nce;
int gpio_iset2;
+ int gpio_2407x_en2;
int state_nce;
int state_iset2;
+ int state_2407x_en2;
+
+ int max_uA;
};
static int bq24022_set_current_limit(struct regulator_dev *rdev,
@@ -35,12 +40,30 @@ static int bq24022_set_current_limit(struct regulator_dev *rdev,
{
struct bq24022 *bq = rdev_get_drvdata(rdev);
- dev_dbg(rdev_get_dev(rdev), "setting current limit to %s mA\n",
- max_uA >= 500000 ? "500" : "100");
+ if (bq->max_uA && bq->max_uA > 500000
+ && max_uA >= bq->max_uA && bq->gpio_2407x_en2) {
+ dev_dbg(rdev_get_dev(rdev),
+ "setting current limit to %d mA\n",
+ bq->max_uA / 1000);
+ gpio_set_value(bq->gpio_2407x_en2, 1);
+ bq->state_2407x_en2 = 1;
+ gpio_set_value(bq->gpio_iset2, 0);
+ bq->state_iset2 = 0;
+ } else if (max_uA >= 100000) {
+ dev_dbg(rdev_get_dev(rdev), "setting current limit to %s mA\n",
+ max_uA >= 500000 ? "500" : "100");
+ if (bq->gpio_2407x_en2) {
+ gpio_set_value(bq->gpio_2407x_en2, 0);
+ bq->state_2407x_en2 = 0;
+ }
+ bq->state_iset2 = (max_uA >= 500000);
+ gpio_set_value(bq->gpio_iset2, bq->state_iset2);
+ } else {
+ dev_err(rdev_get_dev(rdev), "cannot set current limit below 100 mA\n");
+ return -EINVAL;
+ }
/* REVISIT: maybe return error if min_uA != 0 ? */
- bq->state_iset2 = (max_uA >= 500000);
- gpio_set_value(bq->gpio_iset2, bq->state_iset2);
return 0;
}
@@ -48,7 +71,10 @@ static int bq24022_get_current_limit(struct regulator_dev *rdev)
{
struct bq24022 *bq = rdev_get_drvdata(rdev);
- return bq->state_iset2 ? 500000 : 100000;
+ if (bq->state_2407x_en2)
+ return bq->state_iset2 ? 0 : bq->max_uA;
+ else
+ return bq->state_iset2 ? 500000 : 100000;
}
static int bq24022_enable(struct regulator_dev *rdev)
@@ -122,8 +148,25 @@ static int __init bq24022_probe(struct platform_device *pdev)
pdata->gpio_iset2);
goto err_iset2;
}
+ if (pdata->gpio_2407x_en2) {
+ ret = gpio_request(pdata->gpio_2407x_en2, "charge_mode2");
+ if (ret) {
+ dev_err(&pdev->dev, "couldn't request EN2 GPIO: %d\n",
+ pdata->gpio_2407x_en2);
+ goto err_en2;
+ }
+ }
/* set initial current to 100mA and disable regulator */
+ if (pdata->gpio_2407x_en2) {
+ ret = gpio_direction_output(pdata->gpio_2407x_en2, 0);
+ if (ret) {
+ dev_err(&pdev->dev, "couldn't set EN2 GPIO: %d\n",
+ pdata->gpio_2407x_en2);
+ goto err_reg;
+ }
+ bq->gpio_2407x_en2 = pdata->gpio_2407x_en2;
+ }
ret = gpio_direction_output(pdata->gpio_iset2, 0);
if (ret) {
dev_err(&pdev->dev, "couldn't set ISET2 GPIO: %d\n",
@@ -140,6 +183,13 @@ static int __init bq24022_probe(struct platform_device *pdev)
bq->gpio_nce = pdata->gpio_nce;
bq->state_nce = 1;
+ /* get maximum current from regulator_init_data */
+ if (pdata->init_data) {
+ bq->max_uA = pdata->init_data->constraints.max_uA;
+ dev_dbg(&pdev->dev, "maximum current is %d mA\n",
+ bq->max_uA / 1000);
+ }
+
bq->rdev = regulator_register(&bq24022_desc, &pdev->dev,
pdata->init_data, bq);
if (IS_ERR(bq->rdev)) {
@@ -152,6 +202,9 @@ static int __init bq24022_probe(struct platform_device *pdev)
return 0;
err_reg:
+ if (pdata->gpio_2407x_en2)
+ gpio_free(pdata->gpio_2407x_en2);
+err_en2:
gpio_free(pdata->gpio_iset2);
err_iset2:
gpio_free(pdata->gpio_nce);
@@ -164,6 +217,8 @@ static int __devexit bq24022_remove(struct platform_device *pdev)
struct bq24022 *bq = platform_get_drvdata(pdev);
regulator_unregister(bq->rdev);
+ if (bq->gpio_2407x_en2)
+ gpio_free(bq->gpio_2407x_en2);
gpio_free(bq->gpio_iset2);
gpio_free(bq->gpio_nce);
diff --git a/include/linux/regulator/bq24022.h b/include/linux/regulator/bq24022.h
index a6d0140..39a48e1 100644
--- a/include/linux/regulator/bq24022.h
+++ b/include/linux/regulator/bq24022.h
@@ -16,9 +16,16 @@ struct regulator_init_data;
* bq24022_mach_info - platform data for bq24022
* @gpio_nce: GPIO line connected to the nCE pin, used to enable / disable charging
* @gpio_iset2: GPIO line connected to the ISET2 pin, used to limit charging current to 100 mA / 500 mA
+ * @gpio_2407x_en2: GPIO line connected to the en2 pin of the bq2407x family
+ * Modes of operation for bq2407x:
+ * EN2 = 0, ISET2 = 0: 100mA
+ * EN2 = 0, ISET2 = 1: 500mA
+ * EN2 = 1, ISET2 = 0: max_current
+ * EN2 = 1, ISET2 = 1: Standby (usb suspend)
*/
struct bq24022_mach_info {
int gpio_nce;
int gpio_iset2;
+ int gpio_2407x_en2;
struct regulator_init_data *init_data;
};
--
1.7.2.3
^ permalink raw reply related
* [PATCH 3/4] bq24022: Keep track of gpio states
From: Heiko Stübner @ 2011-08-29 15:58 UTC (permalink / raw)
To: Mark Brown, Liam Girdwood; +Cc: linux-pm, Philipp Zabel
In-Reply-To: <201108291755.15701.heiko@sntech.de>
gpio_get_value is not definied for output-gpios and can therefore not be
used for the is_enabled and get_current_limit methods of the bq24022.
This patch introduces a private struct to keep track of the values set.
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
drivers/regulator/bq24022.c | 67 +++++++++++++++++++++++++++++-------------
1 files changed, 46 insertions(+), 21 deletions(-)
diff --git a/drivers/regulator/bq24022.c b/drivers/regulator/bq24022.c
index 7bd906c..f75cd07 100644
--- a/drivers/regulator/bq24022.c
+++ b/drivers/regulator/bq24022.c
@@ -15,56 +15,69 @@
#include <linux/platform_device.h>
#include <linux/err.h>
#include <linux/module.h>
+#include <linux/slab.h>
#include <linux/gpio.h>
#include <linux/regulator/bq24022.h>
#include <linux/regulator/driver.h>
+struct bq24022 {
+ struct regulator_dev *rdev;
+
+ int gpio_nce;
+ int gpio_iset2;
+
+ int state_nce;
+ int state_iset2;
+};
static int bq24022_set_current_limit(struct regulator_dev *rdev,
int min_uA, int max_uA)
{
- struct bq24022_mach_info *pdata = rdev_get_drvdata(rdev);
+ struct bq24022 *bq = rdev_get_drvdata(rdev);
dev_dbg(rdev_get_dev(rdev), "setting current limit to %s mA\n",
max_uA >= 500000 ? "500" : "100");
/* REVISIT: maybe return error if min_uA != 0 ? */
- gpio_set_value(pdata->gpio_iset2, max_uA >= 500000);
+ bq->state_iset2 = (max_uA >= 500000);
+ gpio_set_value(bq->gpio_iset2, bq->state_iset2);
return 0;
}
static int bq24022_get_current_limit(struct regulator_dev *rdev)
{
- struct bq24022_mach_info *pdata = rdev_get_drvdata(rdev);
+ struct bq24022 *bq = rdev_get_drvdata(rdev);
- return gpio_get_value(pdata->gpio_iset2) ? 500000 : 100000;
+ return bq->state_iset2 ? 500000 : 100000;
}
static int bq24022_enable(struct regulator_dev *rdev)
{
- struct bq24022_mach_info *pdata = rdev_get_drvdata(rdev);
+ struct bq24022 *bq = rdev_get_drvdata(rdev);
dev_dbg(rdev_get_dev(rdev), "enabling charger\n");
- gpio_set_value(pdata->gpio_nce, 0);
+ gpio_set_value(bq->gpio_nce, 0);
+ bq->state_nce = 0;
return 0;
}
static int bq24022_disable(struct regulator_dev *rdev)
{
- struct bq24022_mach_info *pdata = rdev_get_drvdata(rdev);
+ struct bq24022 *bq = rdev_get_drvdata(rdev);
dev_dbg(rdev_get_dev(rdev), "disabling charger\n");
- gpio_set_value(pdata->gpio_nce, 1);
+ gpio_set_value(bq->gpio_nce, 1);
+ bq->state_nce = 1;
return 0;
}
static int bq24022_is_enabled(struct regulator_dev *rdev)
{
- struct bq24022_mach_info *pdata = rdev_get_drvdata(rdev);
+ struct bq24022 *bq = rdev_get_drvdata(rdev);
- return !gpio_get_value(pdata->gpio_nce);
+ return !bq->state_nce;
}
static struct regulator_ops bq24022_ops = {
@@ -85,12 +98,18 @@ static struct regulator_desc bq24022_desc = {
static int __init bq24022_probe(struct platform_device *pdev)
{
struct bq24022_mach_info *pdata = pdev->dev.platform_data;
- struct regulator_dev *bq24022;
+ struct bq24022 *bq;
int ret;
if (!pdata || !pdata->gpio_nce || !pdata->gpio_iset2)
return -EINVAL;
+ bq = kzalloc(sizeof(struct bq24022), GFP_KERNEL);
+ if (!bq) {
+ dev_err(&pdev->dev, "cannot allocate memory\n");
+ return -ENOMEM;
+ }
+
ret = gpio_request(pdata->gpio_nce, "ncharge_en");
if (ret) {
dev_err(&pdev->dev, "couldn't request nCE GPIO: %d\n",
@@ -103,27 +122,32 @@ static int __init bq24022_probe(struct platform_device *pdev)
pdata->gpio_iset2);
goto err_iset2;
}
+
+ /* set initial current to 100mA and disable regulator */
ret = gpio_direction_output(pdata->gpio_iset2, 0);
if (ret) {
dev_err(&pdev->dev, "couldn't set ISET2 GPIO: %d\n",
pdata->gpio_iset2);
goto err_reg;
}
+ bq->gpio_iset2 = pdata->gpio_iset2;
ret = gpio_direction_output(pdata->gpio_nce, 1);
if (ret) {
dev_err(&pdev->dev, "couldn't set nCE GPIO: %d\n",
pdata->gpio_nce);
goto err_reg;
}
+ bq->gpio_nce = pdata->gpio_nce;
+ bq->state_nce = 1;
- bq24022 = regulator_register(&bq24022_desc, &pdev->dev,
- pdata->init_data, pdata);
- if (IS_ERR(bq24022)) {
+ bq->rdev = regulator_register(&bq24022_desc, &pdev->dev,
+ pdata->init_data, bq);
+ if (IS_ERR(bq->rdev)) {
dev_err(&pdev->dev, "couldn't register regulator\n");
- ret = PTR_ERR(bq24022);
+ ret = PTR_ERR(bq->rdev);
goto err_reg;
}
- platform_set_drvdata(pdev, bq24022);
+ platform_set_drvdata(pdev, bq);
dev_dbg(&pdev->dev, "registered regulator\n");
return 0;
@@ -137,12 +161,13 @@ err_ce:
static int __devexit bq24022_remove(struct platform_device *pdev)
{
- struct bq24022_mach_info *pdata = pdev->dev.platform_data;
- struct regulator_dev *bq24022 = platform_get_drvdata(pdev);
+ struct bq24022 *bq = platform_get_drvdata(pdev);
- regulator_unregister(bq24022);
- gpio_free(pdata->gpio_iset2);
- gpio_free(pdata->gpio_nce);
+ regulator_unregister(bq->rdev);
+ gpio_free(bq->gpio_iset2);
+ gpio_free(bq->gpio_nce);
+
+ kfree(bq);
return 0;
}
--
1.7.2.3
^ permalink raw reply related
* [PATCH 2/4] bq24022: Use dev_err instead of dev_dbg for error messages
From: Heiko Stübner @ 2011-08-29 15:57 UTC (permalink / raw)
To: Mark Brown, Liam Girdwood; +Cc: linux-pm, Philipp Zabel
In-Reply-To: <201108291755.15701.heiko@sntech.de>
This makes error messages visible to the user,
so he/she can eventually fix them.
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
drivers/regulator/bq24022.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/regulator/bq24022.c b/drivers/regulator/bq24022.c
index 56627d7..7bd906c 100644
--- a/drivers/regulator/bq24022.c
+++ b/drivers/regulator/bq24022.c
@@ -93,13 +93,13 @@ static int __init bq24022_probe(struct platform_device *pdev)
ret = gpio_request(pdata->gpio_nce, "ncharge_en");
if (ret) {
- dev_dbg(&pdev->dev, "couldn't request nCE GPIO: %d\n",
+ dev_err(&pdev->dev, "couldn't request nCE GPIO: %d\n",
pdata->gpio_nce);
goto err_ce;
}
ret = gpio_request(pdata->gpio_iset2, "charge_mode");
if (ret) {
- dev_dbg(&pdev->dev, "couldn't request ISET2 GPIO: %d\n",
+ dev_err(&pdev->dev, "couldn't request ISET2 GPIO: %d\n",
pdata->gpio_iset2);
goto err_iset2;
}
@@ -119,7 +119,7 @@ static int __init bq24022_probe(struct platform_device *pdev)
bq24022 = regulator_register(&bq24022_desc, &pdev->dev,
pdata->init_data, pdata);
if (IS_ERR(bq24022)) {
- dev_dbg(&pdev->dev, "couldn't register regulator\n");
+ dev_err(&pdev->dev, "couldn't register regulator\n");
ret = PTR_ERR(bq24022);
goto err_reg;
}
--
1.7.2.3
^ permalink raw reply related
* [PATCH v2 1/4] bq24022: Evaluate returns of gpio_direction_output-calls
From: Heiko Stübner @ 2011-08-29 15:56 UTC (permalink / raw)
To: Mark Brown, Liam Girdwood; +Cc: linux-pm, Philipp Zabel
In-Reply-To: <201108291755.15701.heiko@sntech.de>
It wasn't done before.
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
Changes since v1: use dev_err instead of dev_dbg
drivers/regulator/bq24022.c | 10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/drivers/regulator/bq24022.c b/drivers/regulator/bq24022.c
index e24d1b7..56627d7 100644
--- a/drivers/regulator/bq24022.c
+++ b/drivers/regulator/bq24022.c
@@ -104,7 +104,17 @@ static int __init bq24022_probe(struct platform_device *pdev)
goto err_iset2;
}
ret = gpio_direction_output(pdata->gpio_iset2, 0);
+ if (ret) {
+ dev_err(&pdev->dev, "couldn't set ISET2 GPIO: %d\n",
+ pdata->gpio_iset2);
+ goto err_reg;
+ }
ret = gpio_direction_output(pdata->gpio_nce, 1);
+ if (ret) {
+ dev_err(&pdev->dev, "couldn't set nCE GPIO: %d\n",
+ pdata->gpio_nce);
+ goto err_reg;
+ }
bq24022 = regulator_register(&bq24022_desc, &pdev->dev,
pdata->init_data, pdata);
--
1.7.2.3
^ permalink raw reply related
* [PATCH v2 0/4] bq24022: Add support for bq2407x family
From: Heiko Stübner @ 2011-08-29 15:55 UTC (permalink / raw)
To: Mark Brown, Liam Girdwood; +Cc: linux-pm, Philipp Zabel
The bq2407x family is quite similar to the bq24022 charger.
As suggested by MyungJoo Ham it should be possible to implement
support for it inside the bq24022 driver instead of adding a new one.
The first three patches fix small things Mark Brown identified in my
submission of a separate bq2407x driver that also exist in the bq24022
while the fourth implements the bq2407x specifics.
This second version fixes some glitches found by Mark Brown and introduces
the new second patch fixing the other dev_dbg invocations where dev_err
would be appropriate.
Heiko Stuebner (4):
bq24022: Evaluate returns of gpio_direction_output-calls
bq24022: Use dev_err instead of dev_dbg for error messages to make
them visible
bq24022: Keep track of gpio states
bq24022: Add support for the bq2407x family
drivers/regulator/Kconfig | 7 +-
drivers/regulator/bq24022.c | 144
++++++++++++++++++++++++++++++-------
include/linux/regulator/bq24022.h | 7 ++
3 files changed, 128 insertions(+), 30 deletions(-)
--
1.7.2.3
^ permalink raw reply
* [PATCH pm-freezer 4/4] freezer: use lock_task_sighand() in fake_signal_wake_up()
From: Tejun Heo @ 2011-08-29 14:06 UTC (permalink / raw)
To: Rafael J. Wysocki, Oleg Nesterov, Paul Menage
Cc: containers, linux-pm, linux-kernel
In-Reply-To: <20110829140549.GD18871@mtj.dyndns.org>
cgroup_freezer calls freeze_task() without holding tasklist_lock and,
if the task is exiting, its ->sighand may be gone by the time
fake_signal_wake_up() is called. Use lock_task_sighand() instead of
accessing ->sighand directly.
Signed-off-by: Tejun Heo <tj@kernel.org>
Reported-by: Oleg Nesterov <oleg@redhat.com>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: Paul Menage <paul@paulmenage.org>
---
kernel/freezer.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
Index: work/kernel/freezer.c
===================================================================
--- work.orig/kernel/freezer.c
+++ work/kernel/freezer.c
@@ -103,9 +103,10 @@ static void fake_signal_wake_up(struct t
{
unsigned long flags;
- spin_lock_irqsave(&p->sighand->siglock, flags);
- signal_wake_up(p, 0);
- spin_unlock_irqrestore(&p->sighand->siglock, flags);
+ if (lock_task_sighand(p, &flags)) {
+ signal_wake_up(p, 0);
+ unlock_task_sighand(p, &flags);
+ }
}
/**
^ permalink raw reply
* [PATCH pm-freezer 3/4] freezer: check freezing() before leaving FROZEN state
From: Tejun Heo @ 2011-08-29 14:05 UTC (permalink / raw)
To: Rafael J. Wysocki, Oleg Nesterov, Paul Menage
Cc: containers, linux-pm, linux-kernel
In-Reply-To: <20110829140509.GC18871@mtj.dyndns.org>
If another freeze happens before all tasks leave FROZEN state after
being thawed, the freezer can see the existing FROZEN and consider the
tasks to be frozen but they can clear FROZEN without checking the new
freezing(). Check freezing() while holding freezer_lock before
clearing FROZEN.
Signed-off-by: Tejun Heo <tj@kernel.org>
Reported-by: Oleg Nesterov <oleg@redhat.com>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
---
kernel/freezer.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
Index: work/kernel/freezer.c
===================================================================
--- work.orig/kernel/freezer.c
+++ work/kernel/freezer.c
@@ -60,6 +60,7 @@ bool __refrigerator(bool check_kthr_stop
*/
spin_lock_irq(&freezer_lock);
current->flags |= PF_FROZEN;
+refreeze:
spin_unlock_irq(&freezer_lock);
save = current->state;
@@ -78,8 +79,10 @@ bool __refrigerator(bool check_kthr_stop
schedule();
}
- /* leave FROZEN */
+ /* leave FROZEN after checking freezing() holding freezer_lock */
spin_lock_irq(&freezer_lock);
+ if (freezing(current))
+ goto refreeze;
current->flags &= ~PF_FROZEN;
spin_unlock_irq(&freezer_lock);
^ permalink raw reply
* [PATCH pm-freezer 2/4] freezer: set PF_NOFREEZE on a dying task right before TASK_DEAD
From: Tejun Heo @ 2011-08-29 14:05 UTC (permalink / raw)
To: Rafael J. Wysocki, Oleg Nesterov, Paul Menage
Cc: containers, linux-pm, linux-kernel
In-Reply-To: <20110829140418.GB18871@mtj.dyndns.org>
3fb45733df "freezer: make exiting tasks properly unfreezable" removed
clear_freeze_flag() from exit path and set PF_NOFREEZE right after
PTRACE_EVENT_EXIT; however, Oleg pointed out that following exit paths
may cause interaction with device drivers after PM freezer consider
the system frozen.
There's no try_to_freeze() call in the exit path and the only
necessary guarantee is that freezer doesn't hang waiting for zombies.
Set PF_NOFREEZE right before setting tsk->state to TASK_DEAD instead.
Signed-off-by: Tejun Heo <tj@kernel.org>
Reported-by: Oleg Nesterov <oleg@redhat.com>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
---
kernel/exit.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
Index: work/kernel/exit.c
===================================================================
--- work.orig/kernel/exit.c
+++ work/kernel/exit.c
@@ -913,12 +913,6 @@ NORET_TYPE void do_exit(long code)
ptrace_event(PTRACE_EVENT_EXIT, code);
- /*
- * With ptrace notification done, there's no point in freezing from
- * here on. Disallow freezing.
- */
- current->flags |= PF_NOFREEZE;
-
validate_creds_for_do_exit(tsk);
/*
@@ -1044,6 +1038,10 @@ NORET_TYPE void do_exit(long code)
preempt_disable();
exit_rcu();
+
+ /* this task is now dead and freezer should ignore it */
+ current->flags |= PF_NOFREEZE;
+
/* causes final put_task_struct in finish_task_switch(). */
tsk->state = TASK_DEAD;
schedule();
^ permalink raw reply
* [PATCH pm-freezer 1/4] cgroup_freezer: fix freezer->state setting bug in freezer_change_state()
From: Tejun Heo @ 2011-08-29 14:04 UTC (permalink / raw)
To: Rafael J. Wysocki, Oleg Nesterov, Paul Menage
Cc: containers, linux-pm, linux-kernel
d02f52811d0e "cgroup_freezer: prepare for removal of TIF_FREEZE" moved
setting of freezer->state into freezer_change_state(); unfortunately,
while doing so, when it's beginning to freeze tasks, it sets the state
to CGROUP_FROZEN instead of CGROUP_FREEZING ending up skipping the
whole freezing state. Fix it.
Signed-off-by: Tejun Heo <tj@kernel.org>
Reported-by: Oleg Nesterov <oleg@redhat.com>
Cc: Paul Menage <paul@paulmenage.org>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
---
Rafael, these four patches fix the issues spotted by Oleg during
review of the freezer patchset. Please apply them on pm-freezer once
Oleg acks them. Thanks.
kernel/cgroup_freezer.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Index: work/kernel/cgroup_freezer.c
===================================================================
--- work.orig/kernel/cgroup_freezer.c
+++ work/kernel/cgroup_freezer.c
@@ -311,14 +311,14 @@ static int freezer_change_state(struct c
if (goal_state == freezer->state)
goto out;
- freezer->state = goal_state;
-
switch (goal_state) {
case CGROUP_THAWED:
+ freezer->state = CGROUP_THAWED;
atomic_dec(&system_freezing_cnt);
unfreeze_cgroup(cgroup, freezer);
break;
case CGROUP_FROZEN:
+ freezer->state = CGROUP_FREEZING;
atomic_inc(&system_freezing_cnt);
retval = try_to_freeze_cgroup(cgroup, freezer);
break;
^ permalink raw reply
* Re: [PATCH 2/3] bq24022: Keep track of gpio states
From: Mark Brown @ 2011-08-29 9:18 UTC (permalink / raw)
To: Heiko Stübner; +Cc: linux-pm, Liam Girdwood, Philipp Zabel
In-Reply-To: <201108281509.24250.heiko@sntech.de>
On Sun, Aug 28, 2011 at 03:09:23PM +0200, Heiko Stübner wrote:
> gpio_get_value is not definied for output-gpios and can therefore not be
> used for the is_enabled and get_current_limit methods of the bq24022.
>
> This patch introduces a private struct to keep track of the values set.
Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
This also fixes the use of platform data after probe - platform data
should be able to be marked as initdata which may mean that the kernel
discards it after boot.
^ permalink raw reply
* Re: [PATCH 1/3] bq24022: Evaluate returns of gpio_direction_output-calls
From: Mark Brown @ 2011-08-29 9:16 UTC (permalink / raw)
To: Heiko Stübner; +Cc: linux-pm, Liam Girdwood, Philipp Zabel
In-Reply-To: <201108281508.31412.heiko@sntech.de>
On Sun, Aug 28, 2011 at 03:08:30PM +0200, Heiko Stübner wrote:
> ret = gpio_direction_output(pdata->gpio_iset2, 0);
> + if (ret) {
> + dev_dbg(&pdev->dev, "couldn't set ISET2 GPIO: %d\n",
> + pdata->gpio_iset2);
> + goto err_reg;
> + }
> ret = gpio_direction_output(pdata->gpio_nce, 1);
> + if (ret) {
> + dev_dbg(&pdev->dev, "couldn't set nCE GPIO: %d\n",
> + pdata->gpio_nce);
> + goto err_reg;
> + }
If these are fatal errors the log messages shouldn't be dev_dbg() but
should instead be something that will be displayed by default.
^ permalink raw reply
* Re: [PATCH] spi: Fix builderror in spi-pl022.c
From: Linus Walleij @ 2011-08-29 8:41 UTC (permalink / raw)
To: Peter Huewe
Cc: Grant Likely, linux-kernel, spi-devel-general, Russell King,
linux-pm, linux-arm-kernel
In-Reply-To: <1314559619-19923-1-git-send-email-peterhuewe@gmx.de>
On Sun, Aug 28, 2011 at 9:26 PM, Peter Huewe <peterhuewe@gmx.de> wrote:
> This patch fixes a build error, introduced by commit (67fc8b9f, "PM: add
> runtime PM support to core Primecell driver") which unfortunately was a little
> bit incomplete and did contain a typo (11 instead of 22).
> I'm not sure how this patch could have been tested back then, if it
> doesn't even compile ;)
>
> The build failure was:
> drivers/spi/spi-pl022.c:2292: error: 'adev' undeclared (first use in
> this function)
> drivers/spi/spi-pl022.c:2344: error: 'pl022_suspend' undeclared here
> (not in a function)
>
> The build failure appears e.g. for the u8500 and realview defconfig.
>
> LinuxVersion: linux next-20110826
>
> Signed-off-by: Peter Huewe <peterhuewe@gmx.de>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Thanks!
Linus Walleij
^ permalink raw reply
* Re: [PATCH] spi: Fix builderror in spi-pl022.c
From: Russell King - ARM Linux @ 2011-08-28 20:06 UTC (permalink / raw)
To: Peter Huewe
Cc: Grant Likely, linux-kernel, spi-devel-general, linux-pm,
linux-arm-kernel
In-Reply-To: <1314559619-19923-1-git-send-email-peterhuewe@gmx.de>
On Sun, Aug 28, 2011 at 09:26:59PM +0200, Peter Huewe wrote:
> This patch fixes a build error, introduced by commit (67fc8b9f, "PM: add
> runtime PM support to core Primecell driver") which unfortunately was a little
> bit incomplete and did contain a typo (11 instead of 22).
> I'm not sure how this patch could have been tested back then, if it
> doesn't even compile ;)
Maybe it was tested on configs without CONFIG_SUSPEND enabled - which
is highly likely since my ARM development boards don't support S2RAM.
That's the problem with ifdef'ing code. It makes it impossible to
ensure that it's properly tested.
^ permalink raw reply
* Re: 3.1-rc3-git6: Reported regressions from 3.0
From: Linus Torvalds @ 2011-08-28 19:49 UTC (permalink / raw)
To: Dave Jones, Rafael J. Wysocki, Linux Kernel Mailing List,
Maciej Rutecki
In-Reply-To: <20110828193519.GA14132@redhat.com>
On Sun, Aug 28, 2011 at 12:35 PM, Dave Jones <davej@redhat.com> wrote:
> On Sun, Aug 28, 2011 at 08:22:05PM +0200, Rafael J. Wysocki wrote:
>
> > Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=41742
> > Subject : duplicate filename for intel_backlight with the i915 driver
> > Submitter : François Valenduc <francois.valenduc@tvcablenet.be>
> > Date : 2011-08-25 18:51 (4 days old)
> > First-Bad-Commit: http://git.kernel.org/linus/aaa6fd2a004147bf32fce05720938236de3361d9
>
> this should be fixed by b727d20269e8ef1de002bfea8099f5e9db9e9f23
Actually, by a2cc797d2d1a ("i915: do not setup intel_backlight ").
That b727d20269e is just my merge commit that brings in the fix (and
the bogus initialization of 'locked') into mainline.
Linus
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox