On 23 August 2012 12:37, MyungJoo Ham <myungjoo....@samsung.com> wrote:
>> On 20 August 2012 15:15, 함명주 <myungjoo....@samsung.com> wrote:
>> >> Devfreq returns governor predicted frequency as current
>> >> frequency via sysfs interface. But device may not support
>> >> all frequencies that governor predicts. As per the design
>> >> its driver responsibility to maintain current frequency
>> >> at which device is operating. So add a callback in device
>> >> profile to fix this.
>> >>
>> >> Signed-off-by: Rajagopal Venkat <rajagopal.ven...@linaro.org>
>> >
>> > We still need to support "intended frequency".
>> >
>> > The "cur_freq" node is to show the intended frequency.
>> > As told before, you need to make additional API and ABI for the
>> > "actual frequency".
>> >
>> I don't think userspace will be interested in "intended frequency", i.e
>> governor predicted next target frequency unless code is being
>> debugged. More over device may not support all opps that
>> governor predicts. Hence cur_freq node is fixed to show freq
>> at the which device is operating.
>>
>> Let me know if you still think, intended frequency should be
>> exposed to userspace.
>
> Like CPUfreq, (scaling_cur_freq vs cpuinfo_cur_freq) device driver
> developers are often very interested in such values. At least for me,
> such information has been very helpful for production kernels.
>
Ok. I will add new sysfs node in next version.
> Cheers!
> MyungJoo
>
>>
>> >
>> > Cheers!
>> > MyungJoo
>> >
>> >> ---
>> >> drivers/devfreq/devfreq.c | 14 ++++++++++++--
>> >> include/linux/devfreq.h | 6 +++---
>> >> 2 files changed, 15 insertions(+), 5 deletions(-)
>> >>
>> >> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
>> >> index 375b5aa1..798e8ca 100644
>> >> --- a/drivers/devfreq/devfreq.c
>> >> +++ b/drivers/devfreq/devfreq.c
>> >> @@ -176,7 +176,6 @@ struct devfreq *devfreq_add_device(struct device *dev,
>> >> devfreq->dev.release = devfreq_dev_release;
>> >> devfreq->profile = profile;
>> >> devfreq->governor = governor;
>> >> - devfreq->previous_freq = profile->initial_freq;
>> >> devfreq->governor_data = data;
>> >> devfreq->nb.notifier_call = devfreq_notifier_call;
>> >> devfreq->min_freq = profile->min_freq;
>> >> @@ -272,7 +271,18 @@ static ssize_t show_governor(struct device *dev,
>> >> static ssize_t show_freq(struct device *dev,
>> >> struct device_attribute *attr, char *buf)
>> >> {
>> >> - return sprintf(buf, "%lu\n", to_devfreq(dev)->previous_freq);
>> >> + int ret;
>> >> + unsigned long freq;
>> >> + struct devfreq *devfreq = to_devfreq(dev);
>> >> +
>> >> + if (devfreq->profile->get_cur_freq) {
>> >> + ret = devfreq->profile->get_cur_freq(devfreq->dev.parent,
>> >> + &freq);
>> >> + if (!ret)
>> >> + return sprintf(buf, "%lu\n", freq);
>> >> + }
>> >> +
>> >> + return sprintf(buf, "<unknown>");
>> >> }
>> >>
>> >> static ssize_t store_min_freq(struct device *dev, struct
>> >> device_attribute *attr,
>> >> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
>> >> index 7c6517f..43f111f 100644
>> >> --- a/include/linux/devfreq.h
>> >> +++ b/include/linux/devfreq.h
>> >> @@ -76,6 +76,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
>> >> @@ -91,6 +93,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);
>> >> };
>> >>
>> >> @@ -119,7 +122,6 @@ struct devfreq_governor {
>> >> * @nb notifier block used to notify devfreq object that
>> >> it should
>> >> * reevaluate operable frequencies. Devfreq users may use
>> >> * devfreq.nb to the corresponding register notifier call
>> >> chain.
>> >> - * @previous_freq previously configured frequency value.
>> >> * @governor_data Private data of the governor. The devfreq framework
>> >> * does not touch this.
>> >> * @min_freq Limit minimum frequency requested by user (0: none)
>> >> @@ -142,8 +144,6 @@ struct devfreq {
>> >> const struct devfreq_governor *governor;
>> >> struct notifier_block nb;
>> >>
>> >> - unsigned long previous_freq;
>> >> -
>> >> void *governor_data; /* private data for governors */
>> >>
>> >> unsigned long min_freq;
>> >> --
>> >> 1.7.11.3
>> >>
>> >>
>> >>
>> >>
>> >>
>> >>
>> >>
>> >>
>>
>>
>>
>> --
>> Regards,
>> Rajagopal
>>
>>
>>
>>
>>
>>
>>
--
Regards,
Rajagopal
_______________________________________________
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
