linux-hwmon.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH 5/6] hwmon: (k10temp): Don't show Tdie for all Zen/Zen2/Zen3 CPU/APU
@ 2021-08-27 16:48 Guenter Roeck
  2021-08-27 20:19 ` Limonciello, Mario
  0 siblings, 1 reply; 3+ messages in thread
From: Guenter Roeck @ 2021-08-27 16:48 UTC (permalink / raw)
  To: Mario Limonciello
  Cc: Clemens Ladisch, linux-hwmon, Gabriel Craciunescu, Wei Huang

On Thu, Aug 26, 2021 at 01:40:56PM -0500, Mario Limonciello wrote:
> Tdie is an offset calculation that should only be shown when temp_offset
> is actually put into a table.  This is useless to show for all CPU/APU.
> Show it only when necessary.
> 
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>

Applied.

I won't apply the other patches of the series because they either have issues
or need approval from x86/pci maintainers. No reason to hold up this one
or the first patch of the series, though.

Side note: I accepted this patch because it _seems_ like hwinfo64
does something similar. I do hope, though, that the assertion made
in the patch description is correct and that this doesn't miss CPUs
where there is a (logical or real) difference between Tctl and Tdie.

Guenter

> ---
>  drivers/hwmon/k10temp.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/hwmon/k10temp.c b/drivers/hwmon/k10temp.c
> index 41d9c0c0a1f1..e8ec0e36fc3b 100644
> --- a/drivers/hwmon/k10temp.c
> +++ b/drivers/hwmon/k10temp.c
> @@ -435,7 +435,6 @@ static int k10temp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  	} else if (boot_cpu_data.x86 == 0x17 || boot_cpu_data.x86 == 0x18) {
>  		data->temp_adjust_mask = ZEN_CUR_TEMP_RANGE_SEL_MASK;
>  		data->read_tempreg = read_tempreg_nb_zen;
> -		data->show_temp |= BIT(TDIE_BIT);	/* show Tdie */
>  		data->is_zen = true;
>  
>  		switch (boot_cpu_data.x86_model) {
> @@ -457,7 +456,6 @@ static int k10temp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  	} else if (boot_cpu_data.x86 == 0x19) {
>  		data->temp_adjust_mask = ZEN_CUR_TEMP_RANGE_SEL_MASK;
>  		data->read_tempreg = read_tempreg_nb_zen;
> -		data->show_temp |= BIT(TDIE_BIT);
>  		data->is_zen = true;
>  
>  		switch (boot_cpu_data.x86_model) {
> @@ -478,6 +476,7 @@ static int k10temp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  
>  		if (boot_cpu_data.x86 == entry->model &&
>  		    strstr(boot_cpu_data.x86_model_id, entry->id)) {
> +			data->show_temp |= BIT(TDIE_BIT);	/* show Tdie */
>  			data->temp_offset = entry->offset;
>  			break;
>  		}
> -- 
> 2.25.1
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH 5/6] hwmon: (k10temp): Don't show Tdie for all Zen/Zen2/Zen3 CPU/APU
  2021-08-27 16:48 [PATCH 5/6] hwmon: (k10temp): Don't show Tdie for all Zen/Zen2/Zen3 CPU/APU Guenter Roeck
@ 2021-08-27 20:19 ` Limonciello, Mario
  0 siblings, 0 replies; 3+ messages in thread
From: Limonciello, Mario @ 2021-08-27 20:19 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Clemens Ladisch, linux-hwmon, Gabriel Craciunescu, Wei Huang

On 8/27/2021 11:48, Guenter Roeck wrote:
> On Thu, Aug 26, 2021 at 01:40:56PM -0500, Mario Limonciello wrote:
>> Tdie is an offset calculation that should only be shown when temp_offset
>> is actually put into a table.  This is useless to show for all CPU/APU.
>> Show it only when necessary.
>>
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> 
> Applied.

Really appreciate the expedient review.

> 
> I won't apply the other patches of the series because they either have issues
> or need approval from x86/pci maintainers. No reason to hold up this one
> or the first patch of the series, though.

Thanks.  I've sent v2 to accommodate suggested changes and to loop in 
other maintainers on the applicable patches since as you point out some 
files cross subsystems.

> 
> Side note: I accepted this patch because it _seems_ like hwinfo64
> does something similar. I do hope, though, that the assertion made
> in the patch description is correct and that this doesn't miss CPUs
> where there is a (logical or real) difference between Tctl and Tdie.
> 

If there does turn out to be a system that these are different, then 
showing both as the same value will certainly send a confusing message. 
  If such a case is discovered we should be adding to the table (which 
will bring back Tdie for those products).

> Guenter
> 
>> ---
>>   drivers/hwmon/k10temp.c | 3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/hwmon/k10temp.c b/drivers/hwmon/k10temp.c
>> index 41d9c0c0a1f1..e8ec0e36fc3b 100644
>> --- a/drivers/hwmon/k10temp.c
>> +++ b/drivers/hwmon/k10temp.c
>> @@ -435,7 +435,6 @@ static int k10temp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>>   	} else if (boot_cpu_data.x86 == 0x17 || boot_cpu_data.x86 == 0x18) {
>>   		data->temp_adjust_mask = ZEN_CUR_TEMP_RANGE_SEL_MASK;
>>   		data->read_tempreg = read_tempreg_nb_zen;
>> -		data->show_temp |= BIT(TDIE_BIT);	/* show Tdie */
>>   		data->is_zen = true;
>>   
>>   		switch (boot_cpu_data.x86_model) {
>> @@ -457,7 +456,6 @@ static int k10temp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>>   	} else if (boot_cpu_data.x86 == 0x19) {
>>   		data->temp_adjust_mask = ZEN_CUR_TEMP_RANGE_SEL_MASK;
>>   		data->read_tempreg = read_tempreg_nb_zen;
>> -		data->show_temp |= BIT(TDIE_BIT);
>>   		data->is_zen = true;
>>   
>>   		switch (boot_cpu_data.x86_model) {
>> @@ -478,6 +476,7 @@ static int k10temp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>>   
>>   		if (boot_cpu_data.x86 == entry->model &&
>>   		    strstr(boot_cpu_data.x86_model_id, entry->id)) {
>> +			data->show_temp |= BIT(TDIE_BIT);	/* show Tdie */
>>   			data->temp_offset = entry->offset;
>>   			break;
>>   		}
>> -- 
>> 2.25.1
>>


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH 5/6] hwmon: (k10temp): Don't show Tdie for all Zen/Zen2/Zen3 CPU/APU
  2021-08-26 18:40 [PATCH 0/6] Add k10temp support for more client APUs Mario Limonciello
@ 2021-08-26 18:40 ` Mario Limonciello
  0 siblings, 0 replies; 3+ messages in thread
From: Mario Limonciello @ 2021-08-26 18:40 UTC (permalink / raw)
  To: Clemens Ladisch
  Cc: linux-hwmon, Gabriel Craciunescu, Guenter Roeck, Wei Huang,
	Mario Limonciello

Tdie is an offset calculation that should only be shown when temp_offset
is actually put into a table.  This is useless to show for all CPU/APU.
Show it only when necessary.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/hwmon/k10temp.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/hwmon/k10temp.c b/drivers/hwmon/k10temp.c
index 41d9c0c0a1f1..e8ec0e36fc3b 100644
--- a/drivers/hwmon/k10temp.c
+++ b/drivers/hwmon/k10temp.c
@@ -435,7 +435,6 @@ static int k10temp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	} else if (boot_cpu_data.x86 == 0x17 || boot_cpu_data.x86 == 0x18) {
 		data->temp_adjust_mask = ZEN_CUR_TEMP_RANGE_SEL_MASK;
 		data->read_tempreg = read_tempreg_nb_zen;
-		data->show_temp |= BIT(TDIE_BIT);	/* show Tdie */
 		data->is_zen = true;
 
 		switch (boot_cpu_data.x86_model) {
@@ -457,7 +456,6 @@ static int k10temp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	} else if (boot_cpu_data.x86 == 0x19) {
 		data->temp_adjust_mask = ZEN_CUR_TEMP_RANGE_SEL_MASK;
 		data->read_tempreg = read_tempreg_nb_zen;
-		data->show_temp |= BIT(TDIE_BIT);
 		data->is_zen = true;
 
 		switch (boot_cpu_data.x86_model) {
@@ -478,6 +476,7 @@ static int k10temp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 
 		if (boot_cpu_data.x86 == entry->model &&
 		    strstr(boot_cpu_data.x86_model_id, entry->id)) {
+			data->show_temp |= BIT(TDIE_BIT);	/* show Tdie */
 			data->temp_offset = entry->offset;
 			break;
 		}
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-08-27 20:20 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-27 16:48 [PATCH 5/6] hwmon: (k10temp): Don't show Tdie for all Zen/Zen2/Zen3 CPU/APU Guenter Roeck
2021-08-27 20:19 ` Limonciello, Mario
  -- strict thread matches above, loose matches on Subject: below --
2021-08-26 18:40 [PATCH 0/6] Add k10temp support for more client APUs Mario Limonciello
2021-08-26 18:40 ` [PATCH 5/6] hwmon: (k10temp): Don't show Tdie for all Zen/Zen2/Zen3 CPU/APU Mario Limonciello

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).