Patchwork [v3,4/5] powernv:cpufreq: Export nominal frequency via sysfs.

login
register
mail settings
Submitter Gautham R. Shenoy
Date March 20, 2014, 12:10 p.m.
Message ID <1395317460-14811-5-git-send-email-ego@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/332099/
State Changes Requested
Headers show

Comments

Gautham R. Shenoy - March 20, 2014, 12:10 p.m.
From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>

Create a driver attribute named cpuinfo_nominal_freq which
creates a sysfs read-only file named cpuinfo_nominal_freq. Export
the frequency corresponding to the nominal_pstate through this
interface.

Nominal frequency is the highest non-turbo frequency for the
platform.  This is generally used for setting governor policies from
user space for optimal energy efficiency.

Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
---
 drivers/cpufreq/powernv-cpufreq.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)
viresh kumar - March 21, 2014, 8:47 a.m.
On Thu, Mar 20, 2014 at 5:40 PM, Gautham R. Shenoy
<ego@linux.vnet.ibm.com> wrote:
> From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
>
> Create a driver attribute named cpuinfo_nominal_freq which
> creates a sysfs read-only file named cpuinfo_nominal_freq. Export
> the frequency corresponding to the nominal_pstate through this
> interface.
>
> Nominal frequency is the highest non-turbo frequency for the
> platform.  This is generally used for setting governor policies from
> user space for optimal energy efficiency.
>
> Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> ---
>  drivers/cpufreq/powernv-cpufreq.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>
> diff --git a/drivers/cpufreq/powernv-cpufreq.c b/drivers/cpufreq/powernv-cpufreq.c
> index e7b0292..46bee8a 100644
> --- a/drivers/cpufreq/powernv-cpufreq.c
> +++ b/drivers/cpufreq/powernv-cpufreq.c
> @@ -142,8 +142,30 @@ static unsigned int pstate_id_to_freq(int pstate_id)
>         return powernv_freqs[i].frequency;
>  }
>
> +/**
> + * show_cpuinfo_nominal_freq - Show the nominal CPU frequency as indicated by
> + * the firmware
> + */
> +static ssize_t show_cpuinfo_nominal_freq(struct cpufreq_policy *policy,
> +                                       char *buf)
> +{
> +       int nominal_freq;
> +       nominal_freq = pstate_id_to_freq(powernv_pstate_info.pstate_nominal_id);
> +       return sprintf(buf, "%u\n", nominal_freq);

return sprintf(buf, "%u\n",
pstate_id_to_freq(powernv_pstate_info.pstate_nominal_id));

??

> +}
> +
> +

remove extra blank line.

> +struct freq_attr cpufreq_freq_attr_cpuinfo_nominal_freq = {
> +       .attr = { .name = "cpuinfo_nominal_freq",
> +                 .mode = 0444,
> +               },

Align {}

> +       .show = show_cpuinfo_nominal_freq,
> +};
> +
> +
>  static struct freq_attr *powernv_cpu_freq_attr[] = {
>         &cpufreq_freq_attr_scaling_available_freqs,
> +       &cpufreq_freq_attr_cpuinfo_nominal_freq,
>         NULL,
>  };
Gautham R. Shenoy - March 21, 2014, 9:55 a.m.
On Fri, Mar 21, 2014 at 02:17:28PM +0530, Viresh Kumar wrote:
> > +static ssize_t show_cpuinfo_nominal_freq(struct cpufreq_policy *policy,
> > +                                       char *buf)
> > +{
> > +       int nominal_freq;
> > +       nominal_freq = pstate_id_to_freq(powernv_pstate_info.pstate_nominal_id);
> > +       return sprintf(buf, "%u\n", nominal_freq);
> 
> return sprintf(buf, "%u\n",
> pstate_id_to_freq(powernv_pstate_info.pstate_nominal_id));

Sure. Will fix this. 
> 
> ??
> 
> > +}
> > +
> > +
> 
> remove extra blank line.
> 
> > +struct freq_attr cpufreq_freq_attr_cpuinfo_nominal_freq = {
> > +       .attr = { .name = "cpuinfo_nominal_freq",
> > +                 .mode = 0444,
> > +               },
> 
> Align {}

Probably the use of ATTR_RO(cpuinfo_nominal_freq) and renaming
show_cpuinfo_nominal_freq to cpuinfo_nominal_freq_show() would be even
better. What do you think ?

> 
> > +       .show = show_cpuinfo_nominal_freq,
> > +};
> > +
> > +
> >  static struct freq_attr *powernv_cpu_freq_attr[] = {
> >         &cpufreq_freq_attr_scaling_available_freqs,
> > +       &cpufreq_freq_attr_cpuinfo_nominal_freq,
> >         NULL,
> >  };

This needs to be rewritten to include the entries of cpufreq_generic_attr.
>

Thanks for the review.

--
Regards
gautham.
viresh kumar - March 21, 2014, 9:57 a.m.
On 21 March 2014 15:25, Gautham R Shenoy <ego@linux.vnet.ibm.com> wrote:
> Probably the use of ATTR_RO(cpuinfo_nominal_freq) and renaming
> show_cpuinfo_nominal_freq to cpuinfo_nominal_freq_show() would be even
> better. What do you think ?

+1

Patch

diff --git a/drivers/cpufreq/powernv-cpufreq.c b/drivers/cpufreq/powernv-cpufreq.c
index e7b0292..46bee8a 100644
--- a/drivers/cpufreq/powernv-cpufreq.c
+++ b/drivers/cpufreq/powernv-cpufreq.c
@@ -142,8 +142,30 @@  static unsigned int pstate_id_to_freq(int pstate_id)
 	return powernv_freqs[i].frequency;
 }
 
+/**
+ * show_cpuinfo_nominal_freq - Show the nominal CPU frequency as indicated by
+ * the firmware
+ */
+static ssize_t show_cpuinfo_nominal_freq(struct cpufreq_policy *policy,
+					char *buf)
+{
+	int nominal_freq;
+	nominal_freq = pstate_id_to_freq(powernv_pstate_info.pstate_nominal_id);
+	return sprintf(buf, "%u\n", nominal_freq);
+}
+
+
+struct freq_attr cpufreq_freq_attr_cpuinfo_nominal_freq = {
+	.attr = { .name = "cpuinfo_nominal_freq",
+		  .mode = 0444,
+		},
+	.show = show_cpuinfo_nominal_freq,
+};
+
+
 static struct freq_attr *powernv_cpu_freq_attr[] = {
 	&cpufreq_freq_attr_scaling_available_freqs,
+	&cpufreq_freq_attr_cpuinfo_nominal_freq,
 	NULL,
 };