Messages in this thread | | | Subject | Re: [PATCH v2 3/3] PM: Introduce Intel PowerClamp Driver | From | Joe Perches <> | Date | Mon, 26 Nov 2012 15:27:25 -0800 |
| |
On Mon, 2012-11-26 at 06:37 -0800, Jacob Pan wrote: > Intel PowerClamp driver performs synchronized idle injection across > all online CPUs. The goal is to maintain a given package level C-state > ratio.
trivial notes:
> diff --git a/drivers/thermal/intel_powerclamp.c b/drivers/thermal/intel_powerclamp.c
You should still add #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt before any include so that all messages are prefixed with powerclamp:
> + > +/* #define DEBUG */ > + > +#include <linux/module.h> > +#include <linux/kernel.h> [] > +static void adjust_compensation(int target_ratio, unsigned int win) > +{ > + int delta;
It'd be shorter code to use a temporary like
struct powerclamp_calibration_data *d = &cal_data[target_ratio];
> + > + /* > + * adjust compensations if confidence level has not been reached or > + * there are too many wakeups during the last idle injection period, we > + * cannot trust the data for compensation. > + */ > + if (cal_data[target_ratio].confidence >= CONFIDENCE_OK || > + atomic_read(&idle_wakeup_counter) > > + win * num_online_cpus()) > + return; > + > + delta = set_target_ratio - current_ratio; > + /* filter out bad data */ > + if (delta >= 0 && delta <= (1+target_ratio/10)) { > + if (cal_data[target_ratio].steady_comp) > + cal_data[target_ratio].steady_comp = > + roundup(delta+ > + cal_data[target_ratio].steady_comp, > + 2)/2;
so that this fits on a single line and becomes:
if (d->steady_comp) d->steady_comp = roundup(delta + d->steady_comp, 2) / 2; etc.
What clamps target_ratio to the correct range? I briefly scanned the code but didn't spot it.
| |