| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 10 Sep 2012 12:34:20 +0530
From: Rajagopal Venkat <rajagopal.venkat@...aro.org>
To: "Rafael J. Wysocki" <rjw@...k.pl>
Cc: mturquette@...aro.org, myungjoo.ham@...sung.com,
kyungmin.park@...sung.com, patches@...aro.org,
linaro-dev@...ts.linaro.org, linux-pm@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] devfreq: Add current freq callback in device profile
On 10 September 2012 03:30, Rafael J. Wysocki <rjw@...k.pl> wrote:
> On Monday, September 03, 2012, Rajagopal Venkat wrote:
>> Devfreq returns governor predicted frequency as current
>> frequency via sysfs interface. But device may not support
>> all frequencies that governor predicts.
>
> Do you have any examples, even out of the tree?
The exynos4_bus.c devfreq driver supports five opps. But the ondemand
governor can suggest any frequency between min and max(assuming
policy is set). As per design, governors only need to provide recommended
frequency and the corresponding driver will interpret it correctly.
So, the governor recommended frequency and the frequency at which
device is running could be different. This patch attempts to add new sysfs
node for exposing device current running frequency.
>
>> Its driver
>> responsibility to maintain current frequency at which device
>> is operating. Add a callback in device profile to fix this.
>
> Obviously, there are no users of this callback in the tree right now, so it
> may not be regarded as a fix. I'd rather say it's a new mechanism for
> drivers to use if their current frequencies differ from the governor-selected
> ones.
Ok. It will be updated in next version.
>
>> Also add a new sysfs node to expose governor predicted next
>> target frequency.
>>
>> Signed-off-by: Rajagopal Venkat <rajagopal.venkat@...aro.org>
>> ---
>> Documentation/ABI/testing/sysfs-class-devfreq | 7 +++++++
>> drivers/devfreq/devfreq.c | 14 ++++++++++++++
>> include/linux/devfreq.h | 3 +++
>> 3 files changed, 24 insertions(+)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-class-devfreq b/Documentation/ABI/testing/sysfs-class-devfreq
>> index 23d78b5..9df5205 100644
>> --- a/Documentation/ABI/testing/sysfs-class-devfreq
>> +++ b/Documentation/ABI/testing/sysfs-class-devfreq
>> @@ -21,6 +21,13 @@ Description:
>> The /sys/class/devfreq/.../cur_freq shows the current
>> frequency of the corresponding devfreq object.
>>
>> +What: /sys/class/devfreq/.../target_freq
>> +Date: September 2012
>> +Contact: MyungJoo Ham <myungjoo.ham@...sung.com>
>
> It's a bit odd to add someone else as a contact for interfaces that you're
> introducing. I'd need MyungJoo Ham's ACK for that.
>
>> +Description:
>> + The /sys/class/devfreq/.../target_freq shows the next governor
>> + predicted target frequency of the corresponding devfreq object.
>> +
>> What: /sys/class/devfreq/.../central_polling
>> Date: September 2011
>> Contact: MyungJoo Ham <myungjoo.ham@...sung.com>
>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
>> index 3a5f126..55e9046 100644
>> --- a/drivers/devfreq/devfreq.c
>> +++ b/drivers/devfreq/devfreq.c
>> @@ -400,6 +400,19 @@ static ssize_t show_governor(struct device *dev,
>> static ssize_t show_freq(struct device *dev,
>> struct device_attribute *attr, char *buf)
>> {
>> + unsigned long freq;
>> + struct devfreq *devfreq = to_devfreq(dev);
>> +
>> + if (devfreq->profile->get_cur_freq)
>> + if (!devfreq->profile->get_cur_freq(devfreq->dev.parent, &freq))
>
> That should be
>
> + if (devfreq->profile->get_cur_freq
> + && !devfreq->profile->get_cur_freq(devfreq->dev.parent, &freq))
Done.
>
>> + return sprintf(buf, "%lu\n", freq);
>> +
>
> Moreover, I suppose it should fall back to the current behavior if
> .get_cur_freq() is not present.
Agree. Done.
>
>> + return sprintf(buf, "<unknown>");
>> +}
>> +
>> +static ssize_t show_target_freq(struct device *dev,
>> + struct device_attribute *attr, char *buf)
>> +{
>> return sprintf(buf, "%lu\n", to_devfreq(dev)->previous_freq);
>> }
>>
>> @@ -503,6 +516,7 @@ static ssize_t show_max_freq(struct device *dev, struct device_attribute *attr,
>> static struct device_attribute devfreq_attrs[] = {
>> __ATTR(governor, S_IRUGO, show_governor, NULL),
>> __ATTR(cur_freq, S_IRUGO, show_freq, NULL),
>> + __ATTR(target_freq, S_IRUGO, show_target_freq, NULL),
>> __ATTR(polling_interval, S_IRUGO | S_IWUSR, show_polling_interval,
>> store_polling_interval),
>> __ATTR(min_freq, S_IRUGO | S_IWUSR, show_min_freq, store_min_freq),
>> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
>> index 7c7e179..d12ed41 100644
>> --- a/include/linux/devfreq.h
>> +++ b/include/linux/devfreq.h
>> @@ -66,6 +66,8 @@ struct devfreq_dev_status {
>> * explained above with "DEVFREQ_FLAG_*" macros.
>> * @get_dev_status The device should provide the current performance
>> * status to devfreq, which is used by governors.
>> + * @get_cur_freq The device should provide the current frequency
>> + * at which it is operating.
>> * @exit An optional callback that is called when devfreq
>> * is removing the devfreq object due to error or
>> * from devfreq_remove_device() call. If the user
>> @@ -79,6 +81,7 @@ struct devfreq_dev_profile {
>> int (*target)(struct device *dev, unsigned long *freq, u32 flags);
>> int (*get_dev_status)(struct device *dev,
>> struct devfreq_dev_status *stat);
>> + int (*get_cur_freq)(struct device *dev, unsigned long *freq);
>> void (*exit)(struct device *dev);
>> };
>
> Thanks,
> Rafael
--
Regards,
Rajagopal
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists