Re: [PATCH] hwmon: (k10temp) Add per-CCD temperature monitoring for Zen5 Turin

Re: [PATCH] hwmon: (k10temp) Add per-CCD temperature monitoring for Zen5 Turin

[Guenter Roeck]

On 8/10/26 22:04, Gabriel Ford wrote:
> Adds support for per-CCD temperature monitoring on Zen 5 Turin (EPYC 9005)

Nit: s/Adds/Add/

> CPUs, as they fall in a different model range with a larger number of
> maximum CCDs compared to their desktop counterparts. Tested and working
> on an EPYC 9555P.
> 
> Signed-off-by: Gabriel Ford <gabriel@gford.dev>

I'll need confirmation from someone at AMD to confirm that this is correct.

Also, it appears that the patch did not make it into patchwork nor into any
of the mailing lists, and thus it has not been reviewed by Sashiko.
Any idea why this might be the case ?

Thanks,
Guenter

> ---
>   drivers/hwmon/k10temp.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/hwmon/k10temp.c b/drivers/hwmon/k10temp.c
> index de0760dc597d..b882fe09ac1b 100644
> --- a/drivers/hwmon/k10temp.c
> +++ b/drivers/hwmon/k10temp.c
> @@ -207,6 +207,10 @@ static const char *k10temp_temp_label[] = {
>   	"Tccd10",
>   	"Tccd11",
>   	"Tccd12",
> +	"Tccd13",
> +	"Tccd14",
> +	"Tccd15",
> +	"Tccd16"
>   };
>   
>   static int k10temp_read_labels(struct device *dev,
> @@ -515,6 +519,10 @@ static int k10temp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>   		}
>   	} else if (boot_cpu_data.x86 == 0x1a) {
>   		switch (boot_cpu_data.x86_model) {
> +		case 0x00 ... 0x2f:	/* Zen5 Turin */
> +			data->ccd_offset = 0x1F0;
> +			k10temp_get_ccd_support(data, 16);
> +			break;
>   		case 0x40 ... 0x4f:	/* Zen5 Ryzen Desktop */
>   			data->ccd_offset = 0x308;
>   			k10temp_get_ccd_support(data, 8);

[PATCH] hwmon: (k10temp) Add per-CCD temperature monitoring for Zen5 Turin

[Gabriel Ford]

Add support for per-CCD temperature monitoring on Zen 5 Turin (EPYC 9005)
CPUs, as they fall in a different model range with a larger number of
maximum CCDs compared to their desktop counterparts. Tested and working
on an EPYC 9555P.

Signed-off-by: Gabriel Ford <gabriel@gford.dev>
---
 drivers/hwmon/k10temp.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/hwmon/k10temp.c b/drivers/hwmon/k10temp.c
index de0760dc597d..b882fe09ac1b 100644
--- a/drivers/hwmon/k10temp.c
+++ b/drivers/hwmon/k10temp.c
@@ -207,6 +207,10 @@ static const char *k10temp_temp_label[] = {
 	"Tccd10",
 	"Tccd11",
 	"Tccd12",
+	"Tccd13",
+	"Tccd14",
+	"Tccd15",
+	"Tccd16"
 };
 
 static int k10temp_read_labels(struct device *dev,
@@ -515,6 +519,10 @@ static int k10temp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		}
 	} else if (boot_cpu_data.x86 == 0x1a) {
 		switch (boot_cpu_data.x86_model) {
+		case 0x00 ... 0x2f:	/* Zen5 Turin */
+			data->ccd_offset = 0x1F0;
+			k10temp_get_ccd_support(data, 16);
+			break;
 		case 0x40 ... 0x4f:	/* Zen5 Ryzen Desktop */
 			data->ccd_offset = 0x308;
 			k10temp_get_ccd_support(data, 8);
-- 
2.43.0

Re: [PATCH] hwmon: (k10temp) Add per-CCD temperature monitoring for Zen5 Turin

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch adds support for 16 CCDs (Tccd13-Tccd16) on Zen 5 Turin, but fails to expose these new channels to the hwmon subsystem and read handler, rendering the new sensors completely inaccessible.
--

--- Patch [1]: [PATCH] hwmon: (k10temp) Add per-CCD temperature monitoring for Zen5 Turin ---
commit 91c5657242878293503804d18c7b5245b1066ddc
Author: Gabriel Ford <gabriel@gford.dev>

hwmon: (k10temp) Add per-CCD temperature monitoring for Zen5 Turin
    
Add support for per-CCD temperature monitoring on Zen 5 Turin (EPYC 9005)
CPUs, as they fall in a different model range with a larger number of
maximum CCDs compared to their desktop counterparts. Tested and working
on an EPYC 9555P.

> diff --git a/drivers/hwmon/k10temp.c b/drivers/hwmon/k10temp.c
> index de0760dc597da..b882fe09ac1b6 100644
> --- a/drivers/hwmon/k10temp.c
> +++ b/drivers/hwmon/k10temp.c
> @@ -207,6 +207,10 @@ static const char *k10temp_temp_label[] = {
>  	"Tccd10",
>  	"Tccd11",
>  	"Tccd12",
> +	"Tccd13",
> +	"Tccd14",
> +	"Tccd15",
> +	"Tccd16"
>  };

[Severity: Medium]
Will these new sensors actually be exposed and read correctly by the hwmon
subsystem without corresponding updates to k10temp_info and
k10temp_read_temp()?

In k10temp_info, the hwmon_channel_info array dictates the maximum number of
attributes created, but it appears to be missing entries for Tccd13 through
Tccd16:

drivers/hwmon/k10temp.c:k10temp_info[] = {
	HWMON_CHANNEL_INFO(temp,
			   ...
			   HWMON_T_INPUT | HWMON_T_LABEL,
			   HWMON_T_INPUT | HWMON_T_LABEL),
	NULL
};

Additionally, in k10temp_read_temp(), the switch statement handling raw CCD
reads is restricted to channels 2 through 13:

drivers/hwmon/k10temp.c:k10temp_read_temp() {
    ...
		case 2 ... 13:		/* Tccd{1-12} */
			ret = read_ccd_temp_reg(data, channel - 2, &regval);

			if (ret)
				return ret;

			*val = (regval & ZEN_CCD_TEMP_MASK) * 125 - 49000;
			break;
		default:
			return ret;
    ...
}

Will reading channels 14-17 (Tccd13-16) fall through to the default case and
return -EOPNOTSUPP?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260812163812.2760-1-gabriel@gford.dev?part=1