| Message ID | 20230418153230.679094-1-srinivas.pandruvada@linux.intel.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | platform/x86/intel-uncore-freq: Return error on write frequency | expand |
On Tue, 18 Apr 2023, Srinivas Pandruvada wrote: > Currently when the uncore_write() returns error, it is silently > ignored. Return error to user space when uncore_write() fails. > > Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> > Reviewed-by: Zhang Rui <rui.zhang@intel.com> > Tested-by: Wendy Wang <wendy.wang@intel.com> > --- > This patch has no dependency on TPMI patches for uncore support. > > .../x86/intel/uncore-frequency/uncore-frequency-common.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-common.c b/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-common.c > index cb24de9e97dc..fa8f14c925ec 100644 > --- a/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-common.c > +++ b/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-common.c > @@ -44,14 +44,18 @@ static ssize_t store_min_max_freq_khz(struct uncore_data *data, > int min_max) > { > unsigned int input; > + int ret; > > if (kstrtouint(buf, 10, &input)) > return -EINVAL; > > mutex_lock(&uncore_lock); > - uncore_write(data, input, min_max); > + ret = uncore_write(data, input, min_max); > mutex_unlock(&uncore_lock); > > + if (ret) > + return ret; > + > return count; > } Shouldn't this have Fixes tag? Other than that, Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
On Wed, 2023-04-19 at 16:35 +0300, Ilpo Järvinen wrote: > On Tue, 18 Apr 2023, Srinivas Pandruvada wrote: > > > Currently when the uncore_write() returns error, it is silently > > ignored. Return error to user space when uncore_write() fails. > > > > Signed-off-by: Srinivas Pandruvada > > <srinivas.pandruvada@linux.intel.com> > > Reviewed-by: Zhang Rui <rui.zhang@intel.com> > > Tested-by: Wendy Wang <wendy.wang@intel.com> > > --- > > This patch has no dependency on TPMI patches for uncore support. > > > > .../x86/intel/uncore-frequency/uncore-frequency-common.c | 6 > > +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/platform/x86/intel/uncore-frequency/uncore- > > frequency-common.c b/drivers/platform/x86/intel/uncore- > > frequency/uncore-frequency-common.c > > index cb24de9e97dc..fa8f14c925ec 100644 > > --- a/drivers/platform/x86/intel/uncore-frequency/uncore-frequency- > > common.c > > +++ b/drivers/platform/x86/intel/uncore-frequency/uncore-frequency- > > common.c > > @@ -44,14 +44,18 @@ static ssize_t store_min_max_freq_khz(struct > > uncore_data *data, > > int min_max) > > { > > unsigned int input; > > + int ret; > > > > if (kstrtouint(buf, 10, &input)) > > return -EINVAL; > > > > mutex_lock(&uncore_lock); > > - uncore_write(data, input, min_max); > > + ret = uncore_write(data, input, min_max); > > mutex_unlock(&uncore_lock); > > > > + if (ret) > > + return ret; > > + > > return count; > > } > > Shouldn't this have Fixes tag? With the reorg of the directories, the blame commit is not what which will be shown by "git blame". The original one is: Fixes: 49a474c7ba51 ("platform/x86: Add support for Uncore frequency control") Didn't mark to stable that the current MSR write can't fail on production systems. > > Other than that, > > Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> > > Thanks, Srinivas
Hi, On 4/18/23 17:32, Srinivas Pandruvada wrote: > Currently when the uncore_write() returns error, it is silently > ignored. Return error to user space when uncore_write() fails. > > Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> > Reviewed-by: Zhang Rui <rui.zhang@intel.com> > Tested-by: Wendy Wang <wendy.wang@intel.com> Thank you for your patch, I've applied this patch to my review-hans branch: https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans Note it will show up in my review-hans branch once I've pushed my local branch there, which might take a while. Patches which are added to review-hans now are intended for the next rc1. This branch will get rebased to the next rc1 when it is out and after the rebasing the contents of review-hans will be pushed to the platform-drivers-x86/for-next branch. Regards, Hans > --- > This patch has no dependency on TPMI patches for uncore support. > > .../x86/intel/uncore-frequency/uncore-frequency-common.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-common.c b/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-common.c > index cb24de9e97dc..fa8f14c925ec 100644 > --- a/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-common.c > +++ b/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-common.c > @@ -44,14 +44,18 @@ static ssize_t store_min_max_freq_khz(struct uncore_data *data, > int min_max) > { > unsigned int input; > + int ret; > > if (kstrtouint(buf, 10, &input)) > return -EINVAL; > > mutex_lock(&uncore_lock); > - uncore_write(data, input, min_max); > + ret = uncore_write(data, input, min_max); > mutex_unlock(&uncore_lock); > > + if (ret) > + return ret; > + > return count; > } >
diff --git a/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-common.c b/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-common.c index cb24de9e97dc..fa8f14c925ec 100644 --- a/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-common.c +++ b/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-common.c @@ -44,14 +44,18 @@ static ssize_t store_min_max_freq_khz(struct uncore_data *data, int min_max) { unsigned int input; + int ret; if (kstrtouint(buf, 10, &input)) return -EINVAL; mutex_lock(&uncore_lock); - uncore_write(data, input, min_max); + ret = uncore_write(data, input, min_max); mutex_unlock(&uncore_lock); + if (ret) + return ret; + return count; }