Re: [PATCH v3] PM / devfreq: Add possible_frequencies device attribute

From: MyungJoo Ham <myungjoo.ham@samsung.com>
To: "Saravana Kannan" <skannan@codeaurora.org>,
	박경민 <kyungmin.park@samsung.com>
Cc: "linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-arm-msm@vger.kernel.org" <linux-arm-msm@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v3] PM / devfreq: Add possible_frequencies device attribute
Date: Thu, 17 Jul 2014 00:50:58 +0000 (GMT)	[thread overview]
Message-ID: <725805840.101171405558256488.JavaMail.weblogic@epmlwas01c> (raw)

On Wed, Jul 16, 2014 at 12:01 PM, Saravana Kannan <skannan@codeaurora.org> wrote:
> Some devices use freq_table instead of OPP. For those devices, the
> available_frequencies sysfs file shows up empty. So, add a
> possible_frequencies attribute/syfs file that list all the possible
> frequencies.
>
> For devices that use OPP, the output of this file will match
> available_frequencies. It may change in the future to show all OPP
> frequencies -- even the disabled ones.
>
> Signed-off-by: Saravana Kannan <skannan@codeaurora.org>

Hi,

Please add a documentation entry for this new ABI having a little justification and usage included.

Plus, I am considering to move trans_stat along with this entry to somewhere such as .../stat/*
(you don't need to take care of this.)

Besides, as OPP seems becoming the standard as imagined when devfreq development started,
soon, devfreq may require OPP unless the devfreq device has continuous frequencies.

Cheers,
MyungJoo

> ---
>  drivers/devfreq/devfreq.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
>
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 9f90369..65eed38 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -994,6 +994,31 @@ static ssize_t available_frequencies_show(struct device *d,
>  }
>  static DEVICE_ATTR_RO(available_frequencies);
>
> +static ssize_t possible_frequencies_show(struct device *d,
> +                                         struct device_attribute *attr,
> +                                         char *buf)
> +{
> +       struct devfreq *df = to_devfreq(d);
> +       unsigned int i = 0;
> +       ssize_t count = 0;
> +
> +       if (!df->profile->freq_table)
> +               return available_frequencies_show(d, attr, buf);
> +
> +       for (i = 0; i < df->profile->max_state; i++)
> +               count += scnprintf(&buf[count], (PAGE_SIZE - count - 2),
> +                                  "%u ", df->profile->freq_table[i]);
> +
> +       /* Truncate the trailing space */
> +       if (count)
> +               count--;
> +
> +       count += sprintf(&buf[count], "\n");
> +
> +       return count;
> +}
> +static DEVICE_ATTR_RO(possible_frequencies);
> +
>  static ssize_t trans_stat_show(struct device *dev,
>                                struct device_attribute *attr, char *buf)
>  {
> @@ -1041,6 +1066,7 @@ static struct attribute *devfreq_attrs[] = {
>         &dev_attr_available_governors.attr,
>         &dev_attr_cur_freq.attr,
>         &dev_attr_available_frequencies.attr,
> +       &dev_attr_possible_frequencies.attr,
>         &dev_attr_target_freq.attr,
>         &dev_attr_polling_interval.attr,
>         &dev_attr_min_freq.attr,
> --
> 1.8.2.1
>
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> hosted by The Linux Foundation
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
MyungJoo Ham, Ph.D.
System S/W Lab, S/W Center, Samsung Electronics