Re: [PATCH v2 6/7] dt-bindings: iio: temperature: Add ADT7604 support to adi,ltc2983

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Device Tree hardware representation violation: Fake `adi,sensor-type` values used for new sensors
- [High] Device Tree abstraction violation: Encoding coverage percentage as fake Kelvin temperature
- [Medium] Subsystem Guideline Violation: Missing `adi,adt7604` from generational conditional block
- [Medium] Incomplete schema validation for repurposed sensor types on ADT7604
--

commit 29985f1c91396fb1c61d9d6e8e7e4bc36c38266c
Author: Liviu Stan <liviu.stan@analog.com>

dt-bindings: iio: temperature: Add ADT7604 support to adi,ltc2983

This commit adds device tree bindings support for the ADT7604
multi-sensor temperature system. It introduces new copper trace and leak
detector node types, mapping them to existing hardware capabilities. It also
restricts unsupported node configurations for the ADT7604.

> diff --git a/Documentation/devicetree/bindings/iio/temperature/adi,ltc2983.yaml b/Documentation/devicetree/bindings/iio/temperature/adi,ltc2983.yaml
> index a22725f7619b7..c63b69bd95045 100644
> --- a/Documentation/devicetree/bindings/iio/temperature/adi,ltc2983.yaml
> +++ b/Documentation/devicetree/bindings/iio/temperature/adi,ltc2983.yaml

[ ... ]

> @@ -436,6 +441,115 @@ patternProperties:
>      required:
>        - adi,custom-temp
>  
> +  '^copper-trace@':
> +    $ref: '#/$defs/sensor-node'
> +    unevaluatedProperties: false
> +    description: |
> +      Copper trace resistance sensor (some parts only). Two variants exist:
> +      sub-ohm (< 1 ohm, no custom table allowed) and standard (> 1 ohm,
> +      optional custom table).
> +
> +    properties:
> +      reg:
> +        minimum: 2
> +        maximum: 20
> +
> +      adi,sensor-type:
> +        description: Sensor type for copper trace sensors.
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        const: 32

Is it appropriate to use 32 as the sensor type here?

The commit message indicates the hardware repurposes the custom RTD sensor
type 18 for this. Since the hardware channel assignment field for sensor
types is typically 5 bits with a maximum value of 31, does enforcing 32 in
the device tree leak a virtual driver ID into the ABI rather than describing
the actual hardware registers?

[ ... ]

> +  '^leak-detector@':
> +    $ref: '#/$defs/sensor-node'
> +    unevaluatedProperties: false
> +    description: |
> +      Leak detector sensor (some parts only). Outputs resistance in ohms and,
> +      when a custom table is provided, a coverage percentage via
> +      IIO_COVERAGE_PERCENT (raw/1024 = coverage %).
> +
> +    properties:
> +      reg:
> +        minimum: 2
> +        maximum: 20
> +
> +      adi,sensor-type:
> +        description: Sensor type for leak detector sensors.
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        const: 33

Similar to the copper trace sensor, the commit message states this
repurposes the custom thermistor type 27. Should this be constrained to 27
instead of 33 to match the hardware register value?

[ ... ]

> +      adi,custom-leak-detector:
> +        description: |
> +          Lookup table mapping resistance to coverage data. Entries must be
> +          in ascending resistance order. The coverage data field encodes the
> +          coverage percentage P as (P + 273.15) expressed in uK, i.e.
> +          (P * 1000000 + 273150000).
> +        $ref: /schemas/types.yaml#/definitions/uint64-matrix
> +        minItems: 3
> +        maxItems: 64
> +        items:
> +          items:
> +            - description: Resistance point in uOhms.
> +            - description: Coverage data point (P + 273150000) in uK.

Does this encode non-temperature measurements with arbitrary Kelvin offsets
into the device tree?

It seems this tightly couples the device tree ABI to internal driver
interpolation logic that expects to subtract 273.15. Could the device tree
store the raw coverage percentage mapping instead?

[ ... ]

> @@ -477,6 +591,22 @@ allOf:
>        patternProperties:
>          '^temp@': false
>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: adi,adt7604
> +    then:
> +      patternProperties:
> +        '^thermocouple@': false
> +        '^diode@': false
> +        '^adc@': false
> +        '^temp@': false

The DT bindings subsystem guidelines indicate that when a new compatible
string shares constraints with existing generations, it should be added to
the existing conditional blocks.

Since there is an existing block that already restricts ^temp@ for other
compatible strings, should adi,adt7604 be added to the compatible enum in
that existing block rather than duplicating the ^temp@ restriction here?

Additionally, since types 18 and 27 are repurposed on the ADT7604, does
this schema allow device trees with invalid rtd@ nodes using
adi,sensor-type = <18> or thermistor@ nodes using adi,sensor-type = <27>
to silently pass validation?

Should the then clause also restrict the sensor types allowed within rtd@
and thermistor@ for the ADT7604?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260514144712.64374-1-liviu.stan@analog.com?part=6