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

[sashiko-bot@kernel.org]

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.

> +        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.

[ ... ]
> +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.

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260923-topic-sm8x50-iio-max34417-adc-v1-0-41d4ba1bfc41@linaro.org?part=1