Re: [PATCH 2/2] hwmon: (cgbc-hwmon): Add some temperature sensors

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch adds a 15th voltage sensor ('12V Standby Voltage', mapping to `in14_input`) to the driver's string labels and documentation, but fails to add its corresponding configuration entry to the `cgbc_hwmon_info` array. As a result, the hardware monitoring core will silently ignore the 15th channel and fail to expose it to sysfs.
--

commit 0d30ff3c376bb1134975af10205e094badb07304
Author: Thomas Richard (congatec GmbH) <thomas.richard@bootlin.com>

hwmon: (cgbc-hwmon): Add some temperature sensors

This commit adds several new temperature sensors, updating documentation and
string labels to match the Congatec board controller documentation. It also
modifies existing labels.

> diff --git a/drivers/hwmon/cgbc-hwmon.c b/drivers/hwmon/cgbc-hwmon.c
> index 2effa0b562862..098896ec8c6f6 100644
> --- a/drivers/hwmon/cgbc-hwmon.c
> +++ b/drivers/hwmon/cgbc-hwmon.c
> @@ -41,32 +41,42 @@ enum cgbc_sensor_types {
[ ... ]
>  static const char * const cgbc_hwmon_labels_in[] = {
> -	"CPU Voltage",
> +	"CPU Core Voltage",
>  	"DC Runtime Voltage",
>  	"DC Standby Voltage",
>  	"CMOS Battery Voltage",
> -	"Battery Voltage",
> +	"Battery Supply Voltage",
>  	"AC Voltage",
>  	"Other Voltage",
> -	"5V Voltage",
> +	"5V Runtime Voltage",
>  	"5V Standby Voltage",
> -	"3V3 Voltage",
> +	"3V3 Runtime Voltage",
>  	"3V3 Standby Voltage",
>  	"VCore A Voltage",
>  	"VCore B Voltage",
> -	"12V Voltage",
> +	"12V Runtime Voltage",
> +	"12V Standby Voltage",
>  };

[ ... ]

> @@ -255,7 +265,12 @@ static const struct hwmon_channel_info * const cgbc_hwmon_info[] = {
>  			   HWMON_T_INPUT | HWMON_T_LABEL, HWMON_T_INPUT | HWMON_T_LABEL,
>  			   HWMON_T_INPUT | HWMON_T_LABEL, HWMON_T_INPUT | HWMON_T_LABEL,
>  			   HWMON_T_INPUT | HWMON_T_LABEL, HWMON_T_INPUT | HWMON_T_LABEL,
> -			   HWMON_T_INPUT | HWMON_T_LABEL, HWMON_T_INPUT | HWMON_T_LABEL),
> +			   HWMON_T_INPUT | HWMON_T_LABEL, HWMON_T_INPUT | HWMON_T_LABEL,
> +			   HWMON_T_INPUT | HWMON_T_LABEL, HWMON_T_INPUT | HWMON_T_LABEL,
> +			   HWMON_T_INPUT | HWMON_T_LABEL, HWMON_T_INPUT | HWMON_T_LABEL,
> +			   HWMON_T_INPUT | HWMON_T_LABEL, HWMON_T_INPUT | HWMON_T_LABEL,
> +			   HWMON_T_INPUT | HWMON_T_LABEL, HWMON_T_INPUT | HWMON_T_LABEL,
> +			   HWMON_T_INPUT | HWMON_T_LABEL),
>  	HWMON_CHANNEL_INFO(in,
>  			   HWMON_I_INPUT | HWMON_I_LABEL, HWMON_I_INPUT | HWMON_I_LABEL,
>  			   HWMON_I_INPUT | HWMON_I_LABEL, HWMON_I_INPUT | HWMON_I_LABEL,

[Severity: Medium]
This patch adds a 15th voltage label ("12V Standby Voltage"), but is the
corresponding configuration entry missing from the cgbc_hwmon_info array?

Looking at the 'in' channel info array here, it only has 14 configuration
elements.

Since the hardware monitoring core iterates over the cgbc_hwmon_info
array and stops after 14 entries for 'in', will it silently ignore the 15th
channel and fail to expose it to sysfs?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910-cgbc-hwmon-fix-and-new-sensors-v1-0-ef13ae2fa147@bootlin.com?part=2