Re: [PATCH v3 1/2] dt-bindings: iio: dac: add support for mcp47a1

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The I2C addresses specified in the schema and example appear to be 8-bit write addresses instead of the 7-bit addresses required by the DT specification.
- [Low] The `reg` property schema applies an integer `enum` directly to a matrix array element, which will cause `dt_binding_check` validation to always fail.
--

--- Patch [1]: [PATCH v3 1/2] dt-bindings: iio: dac: add support for mcp47a1 ---
commit 441e840b5a2a3f037c3520c49013a55b7dc4c1bd
Author: Joshua Crofts <[email protected]>

dt-bindings: iio: dac: add support for mcp47a1

The Microchip MCP47A1 is a 6-bit volatile Digital-to-Analog converter
which communicates via I2C.

> diff --git a/Documentation/devicetree/bindings/iio/dac/microchip,mcp47a1.yaml b/Documentation/devicetree/bindings/iio/dac/microchip,mcp47a1.yaml
> new file mode 100644
> index 0000000000000..b181fe73dce84
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/dac/microchip,mcp47a1.yaml
[ ... ]
> +properties:
> +  compatible:
> +    const: microchip,mcp47a1
> +
> +  reg:
> +    items:
> +      - enum: [0x2e, 0x3e]

[Severity: Low]
Will this cause dt_binding_check validation to fail?

Since dt-schema parses reg values into a uint32-matrix (an array of arrays),
the items array applied to it defines a tuple schema for the outer array's
elements. This means the inner array [0x2e] will be matched against the
integer enum [0x2e, 0x3e], which will mathematically fail since an array
is not an integer.

[Severity: Medium]
Are these 8-bit write addresses instead of the 7-bit addresses required by
the Device Tree specification?

If these are indeed 8-bit addresses (as indicated by the 0 LSB and typical
Microchip datasheet conventions), they must be shifted right by 1. That would
make the correct 7-bit DT addresses 0x17 and 0x1f.

> +
> +  vref-supply: true
[ ... ]
> +examples:
> +  - |
> +    i2c {
> +          #address-cells = <1>;
> +          #size-cells = <0>;
> +
> +          dac@2e {
> +              compatible = "microchip,mcp47a1";
> +              reg = <0x2e>;

[Severity: Medium]
Does this example propagate the DT specification violation?

Using an 8-bit write address directly in the reg property and unit name
(dac@2e) will cause the device to fail to probe or communicate, as the I2C
subsystem expects a 7-bit address.

> +              vref-supply = <&vref_regulator>;
> +              vdd-supply = <&vdd_regulator>;
> +          };
> +    };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1