Re: [PATCH v3 1/9] dt-bindings: iio: adc: support the TI ADS126x ADC family

[email protected]
Newsgroups org.kernel.vger.linux-devicetree
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The `interrupt-names` constraint incorrectly uses `enum` directly instead of `items: enum:`, causing schema validation failures.
- [Medium] The `interrupt-names` property is semantically abused to configure hardware routing instead of naming interrupts.
- [Medium] The compatible string `ti,ads1263` lists `ti,ads1262` as a fallback, but they have incompatible IIO provider interfaces due to differing `#io-channel-cells` requirements.
--

--- Patch [1]: [PATCH v3 1/9] dt-bindings: iio: adc: support the TI ADS126x ADC family ---
commit b073c29433e55e8e8b4d7125bae2d50b6eaba149
Author: Kurt Borja <[email protected]>

dt-bindings: iio: adc: support the TI ADS126x ADC family

The commit adds devicetree bindings for the TI ADS1262 and ADS1263 ADCs.
It defines the required and optional properties, including regulators, input
channels, excitation currents, and interrupts.

> diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads1262.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads1262.yaml
> new file mode 100644
> index 0000000000000..c43ba7abb3916
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/ti,ads1262.yaml
> @@ -0,0 +1,379 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/ti,ads1262.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TI ADS1262/ADS1263 analog to digital converter
> +
> +maintainers:
> +  - Kurt Borja <[email protected]>
> +
> +description: |
> +  The ADS1262 and ADS1263 are 38.4-kSPS, delta-sigma (ΔΣ) ADCs with an
> +  integrated PGA, reference, and internal fault monitors. The ADS1263 integrates
> +  an auxiliary, 24-bit, ΔΣ ADC intended for background measurements.
> +
> +  Datasheets:
> +    - ADS126x: https://www.ti.com/lit/ds/symlink/ads1262.pdf
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - const: ti,ads1262
> +      - items:
> +          - const: ti,ads1263
> +          - const: ti,ads1262

[Severity: Medium]
Since ti,ads1263 dictates #io-channel-cells: const: 2 (lower down in the
binding) and ti,ads1262 dictates const: 1, are they truly ABI compatible?

If an older OS driver only knows ti,ads1262, it will expect 1 cell. When
the IIO subsystem attempts to translate a consumer phandle with 2 cells via
of_xlate, it will reject it due to an invalid argument count, breaking all
IIO consumers.

Should ti,ads1263 just be a standalone compatible string rather than using
ti,ads1262 as a fallback?

[ ... ]

> +  interrupt-names:
> +    description:
> +      Specify which pin should be configured as Data Ready interrupt.
> +    enum: [drdy, dout-drdy]

[Severity: High]
Does this schema validation fail for device trees using interrupt-names?

In JSON schema, DT -names properties are implicitly parsed as arrays of
strings (via the core interrupts.yaml meta-schema). By using enum
directly on the array property instead of items: enum:, the schema checks
if the entire array instance (e.g., ["drdy"]) perfectly matches one of the
scalar string values in the enum list. Since an array is never equal to a
string, this will always evaluate to false, breaking schema validation.

[Severity: Medium]
Is interrupt-names being used here to configure hardware routing rather
than identifying interrupts to the OS driver?

The devicetree specification dictates that interrupt-names is strictly used
to map human-readable names to indices in the interrupts array. It must not
be used as a configuration property to tell the driver which physical pin on
the ADC to route a signal to.

Could a custom property (e.g., ti,drdy-pin) be used for hardware routing
configuration instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.