Re: [PATCH 1/2] dt-bindings: iio: add: document the MAX34417 Four-Channel High Dynamic Range Power Accumulator

[Neil Armstrong <neil.armstrong@linaro.org>]

On 9/23/26 21:18, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [Low] dt-bindings: iio: use of vendor-specific rsense property instead of standard
> - [Low] dt-bindings: iio: unnecessary gpio.h header include in example
> - [Low] dt-bindings: iio: contradictory default and required constraints
> --
> 
> commit 83b0b12b383dfd05c3631355160ac1fe40dc6802
> Author: Neil Armstrong <neil.armstrong@linaro.org>
> 
> dt-bindings: iio: add: document the MAX34417 Four-Channel High Dynamic Range Power Accumulator
> 
> This commit documents the MAX34417 power accumulator device tree bindings.
> The device uses an I2C interface to monitor power consumption in portable
> systems.
> 
>> diff --git a/Documentation/devicetree/bindings/iio/adc/maxim,max34417.yaml b/Documentation/devicetree/bindings/iio/adc/maxim,max34417.yaml
>> new file mode 100644
>> index 0000000000000..6f2339a927710
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/adc/maxim,max34417.yaml
> [ ... ]
>> +    properties:
>> +      reg:
>> +        items:
>> +          - minimum: 0
>> +            maximum: 3
>> +
>> +      label: true
>> +
>> +      maxim,rsense-val-micro-ohms:
>> +        description:
>> +          Adjust the Rsense value to the resistor value
>> +        enum: [1000, 2000, 4000, 5000, 10000, 15000, 20000, 25000, 40000, 50000, 100000]
> 
> [Severity: Low]
> Does this binding need the vendor-specific maxim,rsense-val-micro-ohms
> property for the shunt resistor? The standard shunt-resistor-micro-ohms
> property is well-established across multiple kernel subsystems for this
> exact purpose and might be a better fit.
> 
> Additionally, is the hardcoded enum constraint necessary? It artificially
> limits the possible resistor values, whereas physical shunt resistors can be
> of any resistance.

Good points

> 
>> +        default: 1000
>> +
>> +    required:
>> +      - reg
>> +      - maxim,rsense-val-micro-ohms
> 
> [Severity: Low]
> Can the default fallback ever be utilized for maxim,rsense-val-micro-ohms?
> The property is defined with a default of 1000, but is simultaneously listed
> in the required array for the subnode. Because it is required, schema
> validation will fail if the property is missing in a device tree, making the
> default logically dead.

Good point

> 
> [ ... ]
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/gpio/gpio.h>
> 
> [Severity: Low]
> Is the dt-bindings/gpio/gpio.h include needed for the examples block?
> Neither the device tree node nor its child nodes appear to declare or use
> any GPIO properties or constants, so this might just be a copy-paste
> artifact.


Good point

> 
>> +
>> +    i2c {
>> +        #address-cells = <1>;
>