| Message ID | 20230321090410.866766-1-iwona.winiarska@intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | hwmon: (peci/cputemp) Fix miscalculated DTS for SKX | expand |
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
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
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.
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
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",
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",
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(-)