Messages in this thread | | | From | "Rafael J. Wysocki" <> | Date | Thu, 9 Feb 2023 19:59:22 +0100 | Subject | Re: [PATCH] thermal/drivers/intel_powerclamp: Fix module param for duration |
| |
On Wed, Feb 8, 2023 at 8:09 AM Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> wrote: > > After switch to use powercap/idle-inject framework in the Intel > powerclamp driver, the idle "duration" is changed to micro seconds. > > But the module parameter for idle "duration" is in milli seconds. So, > convert to micro seconds in the set callback and convert back to milli > seconds by adding a get callback. > > While here also use mutex protection for setting and getting "duration". > The other uses of "duration" is already protected by mutex. > > Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> > --- > This patch is for linux-pm bleeding-edge branch. Can be after patch: > " > commit 8526eb7fc75abcd09d8bd061610215baf0ca948a > Author: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> > Date: Wed Feb 1 10:28:53 2023 -0800 > > thermal: intel: powerclamp: Use powercap idle-inject feature > " > Can't add Fixes tag as commit ID will change once merged to mainline. > Or wait till the above patch is merged to mainline. > > drivers/thermal/intel/intel_powerclamp.c | 24 ++++++++++++++++++------ > 1 file changed, 18 insertions(+), 6 deletions(-) > > diff --git a/drivers/thermal/intel/intel_powerclamp.c b/drivers/thermal/intel/intel_powerclamp.c > index 1390748706a6..038acccc0509 100644 > --- a/drivers/thermal/intel/intel_powerclamp.c > +++ b/drivers/thermal/intel/intel_powerclamp.c > @@ -74,6 +74,7 @@ static struct thermal_cooling_device *cooling_dev; > > static DEFINE_MUTEX(powerclamp_lock); > > +/* This duration is in micro seconds */ > static unsigned int duration; > static unsigned int pkg_cstate_ratio_cur; > static unsigned int window_size; > @@ -90,23 +91,34 @@ static int duration_set(const char *arg, const struct kernel_param *kp) > pr_err("Out of recommended range %lu, between 6-25ms\n", > new_duration); > ret = -EINVAL; > + goto exit; > } > > - duration = clamp(new_duration, 6ul, 25ul); > - smp_mb(); > - > + mutex_lock(&powerclamp_lock); > + duration = clamp(new_duration, 6ul, 25ul) * 1000; > + mutex_unlock(&powerclamp_lock); > exit: > > return ret; > } > > +static int duration_get(char *buf, const struct kernel_param *kp) > +{ > + int ret; > + > + mutex_lock(&powerclamp_lock); > + ret = sysfs_emit(buf, "%d\n", duration / 1000); > + mutex_unlock(&powerclamp_lock); > + > + return ret; > +} > + > static const struct kernel_param_ops duration_ops = { > .set = duration_set, > - .get = param_get_int, > + .get = duration_get, > }; > > - > -module_param_cb(duration, &duration_ops, &duration, 0644); > +module_param_cb(duration, &duration_ops, NULL, 0644); > MODULE_PARM_DESC(duration, "forced idle time for each attempt in msec."); > > struct powerclamp_calibration_data { > --
Applied with some edits in the subject and changelog, thanks!
| |