> 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.
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
>
>
>
>
>
>
>
_______________________________________________
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
