Re: [PATCH 2/3] hwmon: (pmbus/max34440): add support for newer version of max34451

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Forward compatibility issue in ETNA8+ revision check due to strict equality
--

--- Patch [2]: [PATCH 2/3] hwmon: (pmbus/max34440): add support for newer version of max34451 ---
commit 02e46b67255562393d163820572cc52a0cabe3e7
Author: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>

hwmon: (pmbus/max34440): add support for newer version of max34451

This commit adds support for the max34451etna8+ hardware revision.
It updates the direct format coefficients specifically for this newer
revision.

> diff --git a/drivers/hwmon/pmbus/max34440.c b/drivers/hwmon/pmbus/max34440.c
> index e56057e9273c1..024109df26db5 100644
> --- a/drivers/hwmon/pmbus/max34440.c
> +++ b/drivers/hwmon/pmbus/max34440.c

[ ... ]

> @@ -343,12 +344,24 @@ static int max34451_set_supported_funcs(struct i2c_client *client,
>  		max34451_na6 = true;
>  		data->info.format[PSC_VOLTAGE_IN] = direct;
>  		data->info.format[PSC_CURRENT_IN] = direct;
> -		data->info.m[PSC_VOLTAGE_IN] = 1;
> +
>  		data->info.b[PSC_VOLTAGE_IN] = 0;
> -		data->info.R[PSC_VOLTAGE_IN] = 3;
> -		data->info.m[PSC_CURRENT_IN] = 1;
>  		data->info.b[PSC_CURRENT_IN] = 0;
> -		data->info.R[PSC_CURRENT_IN] = 2;
> +		if (rv == MAX34451ETNA8_MFR_REV) {

[Severity: Medium]
Will this strict equality check cause issues with future hardware revisions?

The commit message mentions adding support for the ETNA8+ version, but if a
subsequent hardware revision probes with a manufacturer revision greater than
MAX34451ETNA8_MFR_REV (e.g., 0x0015), the outer check
rv >= MAX34451ETNA6_MFR_REV will evaluate to true, but this inner check will
fail.

This causes the code to silently fall through to the else block, regressing to
the older ETNA6 coefficients rather than retaining the newer ETNA8 ones.

Could this use >= instead of == to maintain forward compatibility, similar
to how ETNA6 is checked on line 328?

> +			data->info.m[PSC_VOLTAGE_IN] = 125;
> +			data->info.R[PSC_VOLTAGE_IN] = 0;
> +			data->info.m[PSC_VOLTAGE_OUT] = 125;
> +			data->info.R[PSC_VOLTAGE_OUT] = 0;
> +			data->info.m[PSC_CURRENT_IN] = 250;
> +			data->info.R[PSC_CURRENT_IN] = -1;
> +			data->info.m[PSC_CURRENT_OUT] = 250;
> +			data->info.R[PSC_CURRENT_OUT] = -1;
> +		} else {
> +			data->info.m[PSC_VOLTAGE_IN] = 1;
> +			data->info.R[PSC_VOLTAGE_IN] = 3;
> +			data->info.m[PSC_CURRENT_IN] = 1;
> +			data->info.R[PSC_CURRENT_IN] = 2;
> +		}
>  		data->iout_oc_fault_limit = PMBUS_IOUT_OC_FAULT_LIMIT;
>  		data->iout_oc_warn_limit = PMBUS_IOUT_OC_WARN_LIMIT;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716-max34451_fixes-v1-0-a941b27eaecb@analog.com?part=2