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

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 <[email protected]>

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/[email protected]?part=1