Re: [PATCH v5 3/4] dt-bindings: hwmon: (pmbus/max20830): add max20830c and max20840c support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The dt-bindings introduce `adi,max20840c` with a fallback to `adi,max20830`, asserting compatibility, but the existing `max20830` driver explicitly rejects MAX20840C hardware during probe due to a strict `IC_DEVICE_ID` string check.
--

--- Patch [3]: [PATCH v5 3/4] dt-bindings: hwmon: (pmbus/max20830): add max20830c and max20840c support ---
commit 6c40a5eeb4c4be3695032dfafd53c9b3d0333b47
Author: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>

dt-bindings: hwmon: (pmbus/max20830): add max20830c and max20840c support
    
This commit adds compatible strings for MAX20830C and MAX20840C to the MAX20830
PMBus hardware monitoring driver bindings. It uses a fallback to adi,max20830
for the new variants.

> diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml
> index 01b7275f5b341..cc50a4ee44a27 100644
> --- a/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml
> @@ -22,7 +22,13 @@ allOf:
>  
>  properties:
>    compatible:
> -    const: adi,max20830
> +    oneOf:
> +      - const: adi,max20830
> +      - items:
> +          - enum:
> +              - adi,max20830c
> +              - adi,max20840c
> +          - const: adi,max20830

[Severity: Medium]
Will the existing driver actually be able to probe the MAX20840C hardware
using this fallback?

Looking at max20830_probe() in drivers/hwmon/pmbus/max20830.c, there is a
strict device ID string check:

drivers/hwmon/pmbus/max20830.c:max20830_probe() {
    ...
	if (strncmp(buf, "MAX20830", MAX20830_IC_DEVICE_ID_LENGTH - 1))
		return dev_err_probe(&client->dev, -ENODEV,
				     "Unsupported device: '%s'\n", buf);
    ...
}

Will this actively reject a MAX20840C device since its ID presumably starts
with "MAX20840"?

Additionally, the commit message mentions a longer IC_DEVICE_ID, but the
I2C block read in max20830_probe() hardcodes the read length:

drivers/hwmon/pmbus/max20830.c:max20830_probe() {
    ...
		ret = i2c_smbus_read_i2c_block_data(client, PMBUS_IC_DEVICE_ID,
						    MAX20830_IC_DEVICE_ID_LENGTH + 1,
						    buf);
    ...
}

Will the longer ID be truncated before validation?

>  
>    reg:
>      maxItems: 1

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260730-dev-max20830c-v5-0-a7553f84ee74@analog.com?part=3