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 <[email protected]>
Reviewed-by: Zhang Rui <[email protected]>
Tested-by: Wendy Wang <[email protected]>
---
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;
}
--
2.38.1
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 <[email protected]>
> Reviewed-by: Zhang Rui <[email protected]>
> Tested-by: Wendy Wang <[email protected]>
> ---
> 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 <[email protected]>
--
i.
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
> > <[email protected]>
> > Reviewed-by: Zhang Rui <[email protected]>
> > Tested-by: Wendy Wang <[email protected]>
> > ---
> > 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 <[email protected]>
>
>
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 <[email protected]>
> Reviewed-by: Zhang Rui <[email protected]>
> Tested-by: Wendy Wang <[email protected]>
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;
> }
>