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

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

[MyungJoo Ham]

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

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

[Saravana Kannan]

On 07/16/2014 05:50 PM, MyungJoo Ham wrote:
> 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.

Will do.

> 
> 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.)

Ok.

> 
> 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.

I agree. Only one caveat with OPP is that if a device isn't using OPP to
do any voltage scaling, then it forces a voltage column with 0s. Also,
even if we make OPP mandatory, we'll still want trans_stats that
currently seem to depend on freq_table being populated. I was actually
planning on sending out more patches later that'll do a lot of stuff
automatically for devices with OPP. Like creating freq_table, etc.

-Saravana

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

[PATCH v4] PM / devfreq: Add possible_frequencies device attribute

[Saravana Kannan]

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>
---
 Documentation/ABI/testing/sysfs-class-devfreq | 15 +++++++++++++++
 drivers/devfreq/devfreq.c                     | 26 ++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-class-devfreq b/Documentation/ABI/testing/sysfs-class-devfreq
index ee39aca..a461e67 100644
--- a/Documentation/ABI/testing/sysfs-class-devfreq
+++ b/Documentation/ABI/testing/sysfs-class-devfreq
@@ -72,6 +72,21 @@ Description:
 		This is a snapshot of available frequencies and not limited
 		by the min/max frequency restrictions.
 
+What:		/sys/class/devfreq/.../possible_frequencies
+Date:		July 2014
+Contact:	Saravana Kannan <skannan@codeaurora.org>
+Description:
+		The /sys/class/devfreq/.../possible_frequencies shows
+		the possible frequencies of the corresponding devfreq object.
+		This is a snapshot of possible frequencies and not limited by
+		the min/max frequency restrictions. Unlike
+		available_frequencies, this is also does not require the
+		devfreq device to use OPP for listing its possible
+		frequencies. When OPP is used, this behaves the same way as
+		available_frequencies. It maybe improved in the future to
+		list all possible OPP frequencies even if some of them are
+		disabled at run-time.
+
 What:		/sys/class/devfreq/.../available_governors
 Date:		October 2012
 Contact:	Nishanth Menon <nm@ti.com>
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

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

[함명주]

Sender : Saravana Kannan<skannan@codeaurora.org>
Title : [PATCH v4] PM / devfreq: Add possible_frequencies device attribute
>
> 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 

Looks Good.
Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>

I'll rebase and merge into devfreq-next when I get back (I can only access the Internet via mobile phones until Wed this week.)
I'll move this sysfs entry to .../stat along with stat sysfs and make it conditional (appears only when stat is available)

Cheers,
MyungJoo.
--
MyungJoo Ham (함명주), PHD
Frontier CS Lab, Software Center
Samsung Electronics
Cell: +82-10-6714-2858