1. Patchew
  2. linux
  3. View series

[PATCH] hwmon: (peci/cputemp) Fix miscalculated DTS for SKX

Iwona Winiarska posted 1 patch 2 years, 10 months ago 
drivers/hwmon/peci/cputemp.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
[PATCH] hwmon: (peci/cputemp) Fix miscalculated DTS for SKX
Posted by Iwona Winiarska 2 years, 10 months ago 
For Skylake, DTS temperature of the CPU is reported in S10.6 format
instead of S8.8.

Reported-by: Paul Fertser <fercerpav@gmail.com>
Link: https://lore.kernel.org/lkml/ZBhHS7v+98NK56is@home.paul.comp/
Signed-off-by: Iwona Winiarska <iwona.winiarska@intel.com>
---
 drivers/hwmon/peci/cputemp.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/peci/cputemp.c b/drivers/hwmon/peci/cputemp.c
index 30850a479f61..87d56f0fc888 100644
--- a/drivers/hwmon/peci/cputemp.c
+++ b/drivers/hwmon/peci/cputemp.c
@@ -537,6 +537,12 @@ static const struct cpu_info cpu_hsx = {
 	.thermal_margin_to_millidegree = &dts_eight_dot_eight_to_millidegree,
 };
 
+static const struct cpu_info cpu_skx = {
+	.reg		= &resolved_cores_reg_hsx,
+	.min_peci_revision = 0x33,
+	.thermal_margin_to_millidegree = &dts_ten_dot_six_to_millidegree,
+};
+
 static const struct cpu_info cpu_icx = {
 	.reg		= &resolved_cores_reg_icx,
 	.min_peci_revision = 0x40,
@@ -558,7 +564,7 @@ static const struct auxiliary_device_id peci_cputemp_ids[] = {
 	},
 	{
 		.name = "peci_cpu.cputemp.skx",
-		.driver_data = (kernel_ulong_t)&cpu_hsx,
+		.driver_data = (kernel_ulong_t)&cpu_skx,
 	},
 	{
 		.name = "peci_cpu.cputemp.icx",
-- 
2.39.0
Re: [PATCH] hwmon: (peci/cputemp) Fix miscalculated DTS for SKX
Posted by Guenter Roeck 2 years, 10 months ago 
On Tue, Mar 21, 2023 at 10:04:10AM +0100, Iwona Winiarska wrote:
> For Skylake, DTS temperature of the CPU is reported in S10.6 format
> instead of S8.8.
> 
> Reported-by: Paul Fertser <fercerpav@gmail.com>
> Link: https://lore.kernel.org/lkml/ZBhHS7v+98NK56is@home.paul.comp/
> Signed-off-by: Iwona Winiarska <iwona.winiarska@intel.com>

Applied.

Thanks,
Guenter

> ---
>  drivers/hwmon/peci/cputemp.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwmon/peci/cputemp.c b/drivers/hwmon/peci/cputemp.c
> index 30850a479f61..87d56f0fc888 100644
> --- a/drivers/hwmon/peci/cputemp.c
> +++ b/drivers/hwmon/peci/cputemp.c
> @@ -537,6 +537,12 @@ static const struct cpu_info cpu_hsx = {
>  	.thermal_margin_to_millidegree = &dts_eight_dot_eight_to_millidegree,
>  };
>  
> +static const struct cpu_info cpu_skx = {
> +	.reg		= &resolved_cores_reg_hsx,
> +	.min_peci_revision = 0x33,
> +	.thermal_margin_to_millidegree = &dts_ten_dot_six_to_millidegree,
> +};
> +
>  static const struct cpu_info cpu_icx = {
>  	.reg		= &resolved_cores_reg_icx,
>  	.min_peci_revision = 0x40,
> @@ -558,7 +564,7 @@ static const struct auxiliary_device_id peci_cputemp_ids[] = {
>  	},
>  	{
>  		.name = "peci_cpu.cputemp.skx",
> -		.driver_data = (kernel_ulong_t)&cpu_hsx,
> +		.driver_data = (kernel_ulong_t)&cpu_skx,
>  	},
>  	{
>  		.name = "peci_cpu.cputemp.icx",
Re: [PATCH] hwmon: (peci/cputemp) Fix miscalculated DTS for SKX
Posted by Paul Menzel 2 years, 10 months ago 
Dear Iwona,


Am 21.03.23 um 10:04 schrieb Iwona Winiarska:
> For Skylake, DTS temperature of the CPU is reported in S10.6 format
> instead of S8.8.
> 
> Reported-by: Paul Fertser <fercerpav@gmail.com>
> Link: https://lore.kernel.org/lkml/ZBhHS7v+98NK56is@home.paul.comp/
> Signed-off-by: Iwona Winiarska <iwona.winiarska@intel.com>
> ---
>   drivers/hwmon/peci/cputemp.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwmon/peci/cputemp.c b/drivers/hwmon/peci/cputemp.c
> index 30850a479f61..87d56f0fc888 100644
> --- a/drivers/hwmon/peci/cputemp.c
> +++ b/drivers/hwmon/peci/cputemp.c
> @@ -537,6 +537,12 @@ static const struct cpu_info cpu_hsx = {
>   	.thermal_margin_to_millidegree = &dts_eight_dot_eight_to_millidegree,
>   };
>   
> +static const struct cpu_info cpu_skx = {
> +	.reg		= &resolved_cores_reg_hsx,

This is not aligned. Why not only use one space before the equal sign?

> +	.min_peci_revision = 0x33,
> +	.thermal_margin_to_millidegree = &dts_ten_dot_six_to_millidegree,
> +};
> +
>   static const struct cpu_info cpu_icx = {
>   	.reg		= &resolved_cores_reg_icx,
>   	.min_peci_revision = 0x40,
> @@ -558,7 +564,7 @@ static const struct auxiliary_device_id peci_cputemp_ids[] = {
>   	},
>   	{
>   		.name = "peci_cpu.cputemp.skx",
> -		.driver_data = (kernel_ulong_t)&cpu_hsx,
> +		.driver_data = (kernel_ulong_t)&cpu_skx,
>   	},
>   	{
>   		.name = "peci_cpu.cputemp.icx",


Kind regards,

Paul
Re: [PATCH] hwmon: (peci/cputemp) Fix miscalculated DTS for SKX
Posted by Winiarska, Iwona 2 years, 10 months ago 
On Tue, 2023-03-21 at 10:41 +0100, Paul Menzel wrote:
> Dear Iwona,
> 
> 
> Am 21.03.23 um 10:04 schrieb Iwona Winiarska:
> > For Skylake, DTS temperature of the CPU is reported in S10.6 format
> > instead of S8.8.
> > 
> > Reported-by: Paul Fertser <fercerpav@gmail.com>
> > Link: https://lore.kernel.org/lkml/ZBhHS7v+98NK56is@home.paul.comp/
> > Signed-off-by: Iwona Winiarska <iwona.winiarska@intel.com>
> > ---
> >   drivers/hwmon/peci/cputemp.c | 8 +++++++-
> >   1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/hwmon/peci/cputemp.c b/drivers/hwmon/peci/cputemp.c
> > index 30850a479f61..87d56f0fc888 100644
> > --- a/drivers/hwmon/peci/cputemp.c
> > +++ b/drivers/hwmon/peci/cputemp.c
> > @@ -537,6 +537,12 @@ static const struct cpu_info cpu_hsx = {
> >         .thermal_margin_to_millidegree =
> > &dts_eight_dot_eight_to_millidegree,
> >   };
> >   
> > +static const struct cpu_info cpu_skx = {
> > +       .reg            = &resolved_cores_reg_hsx,
> 
> This is not aligned. Why not only use one space before the equal sign?

Yeah - same alignment problem is present in cpu_hsx and cpu_icx though, so I
just followed along for skx to not stand out visually.
So while I agree that alignment is broken here, I think it might be better to
separate out the potential cleanup from the fix.

Thanks
-Iwona 

> 
> > +       .min_peci_revision = 0x33,
> > +       .thermal_margin_to_millidegree = &dts_ten_dot_six_to_millidegree,
> > +};
> > +
> >   static const struct cpu_info cpu_icx = {
> >         .reg            = &resolved_cores_reg_icx,
> >         .min_peci_revision = 0x40,
> > @@ -558,7 +564,7 @@ static const struct auxiliary_device_id
> > peci_cputemp_ids[] = {
> >         },
> >         {
> >                 .name = "peci_cpu.cputemp.skx",
> > -               .driver_data = (kernel_ulong_t)&cpu_hsx,
> > +               .driver_data = (kernel_ulong_t)&cpu_skx,
> >         },
> >         {
> >                 .name = "peci_cpu.cputemp.icx",
> 
> 
> Kind regards,
> 
> Paul
Re: [PATCH] hwmon: (peci/cputemp) Fix miscalculated DTS for SKX
Posted by andriy.shevchenko@linux.intel.com 2 years, 10 months ago 
On Tue, Mar 21, 2023 at 10:29:22AM +0000, Winiarska, Iwona wrote:
> On Tue, 2023-03-21 at 10:41 +0100, Paul Menzel wrote:
> > Am 21.03.23 um 10:04 schrieb Iwona Winiarska:

...

> > This is not aligned. Why not only use one space before the equal sign?
> 
> Yeah - same alignment problem is present in cpu_hsx and cpu_icx though, so I
> just followed along for skx to not stand out visually.
> So while I agree that alignment is broken here, I think it might be better to
> separate out the potential cleanup from the fix.

I agree with Iwona. If community wants a cleanup, it can be created in
a separate patch. For the fixes it's better to reduce the unrelated churn.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH] hwmon: (peci/cputemp) Fix miscalculated DTS for SKX
Posted by Guenter Roeck 2 years, 10 months ago 
On Tue, Mar 21, 2023 at 03:08:45PM +0200, andriy.shevchenko@linux.intel.com wrote:
> On Tue, Mar 21, 2023 at 10:29:22AM +0000, Winiarska, Iwona wrote:
> > On Tue, 2023-03-21 at 10:41 +0100, Paul Menzel wrote:
> > > Am 21.03.23 um 10:04 schrieb Iwona Winiarska:
> 
> ...
> 
> > > This is not aligned. Why not only use one space before the equal sign?
> > 
> > Yeah - same alignment problem is present in cpu_hsx and cpu_icx though, so I
> > just followed along for skx to not stand out visually.
> > So while I agree that alignment is broken here, I think it might be better to
> > separate out the potential cleanup from the fix.
> 
> I agree with Iwona. If community wants a cleanup, it can be created in
> a separate patch. For the fixes it's better to reduce the unrelated churn.
> 

I don't want a cleanup. The original author chose the alignment,
I accepted it because I give submitters some slack when it comes to
formatting as long as checkpatch doesn't complain, and I do not want
to get into lets-change-alignment wars.

Thanks,
Guenter

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.