Re: [PATCH v15 02/22] dt-bindings: media: i2c: max96717: add support for I2C ATR

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] The `minimum: 0` and `maximum: 1` constraints applied to the `reg` property inside `i2c-atr` child nodes are ineffective due to a type mismatch.
- [Medium] The newly introduced `i2c-atr` subnode fails to mandate `#address-cells` and `#size-cells` in its `required` list.
- [Low] The `anyOf` block handling `i2c-atr` and `i2c-gate` contains redundant logic due to a misunderstanding of how JSON schema validates required properties.
--

--- Patch [2]: [PATCH v15 02/22] dt-bindings: media: i2c: max96717: add support for I2C ATR ---
commit 2370247b1c65cec955ac152aadb6f3da419613c2
Author: Cosmin Tanislav <demonsingur@gmail.com>

dt-bindings: media: i2c: max96717: add support for I2C ATR

MAX96717 is capable of address translation for the connected I2C slaves.
Add support for I2C ATR while keeping I2C gate for compatibility to
support this usecase.

> diff --git a/Documentation/devicetree/bindings/media/i2c/maxim,max96717.yaml b/Documentation/devicetree/bindings/media/i2c/maxim,max96717.yaml
> index d1e8ba6e368ec..976f6dd2c164c 100644
> --- a/Documentation/devicetree/bindings/media/i2c/maxim,max96717.yaml
> +++ b/Documentation/devicetree/bindings/media/i2c/maxim,max96717.yaml
> @@ -91,6 +91,30 @@ properties:
>        incoming GMSL2 link. Therefore, it supports an i2c-gate
>        subnode to configure a sensor.
>  
> +  i2c-alias-pool:
> +    maxItems: 2
> +
> +  i2c-atr:
> +    type: object
> +    additionalProperties: false
> +
> +    properties:
> +      '#address-cells':
> +        const: 1
> +
> +      '#size-cells':
> +        const: 0

[Severity: Medium]
Are #address-cells and #size-cells supposed to be mandatory for this node?

Without a required list in the i2c-atr object, a device tree could omit these
properties entirely and still pass dt-schema validation, even though they are
necessary to parse the reg properties of the i2c@0 and i2c@1 child nodes.

> +
> +    patternProperties:
> +      '^i2c@[01]$':
> +        $ref: /schemas/i2c/i2c-controller.yaml#
> +        unevaluatedProperties: false
> +        properties:
> +          reg:
> +            items:
> +              minimum: 0
> +              maximum: 1

[Severity: Medium]
Will these constraints actually apply to the integer values?

Because dt-schema parses reg as a matrix (an array of arrays), placing
minimum: 0 and maximum: 1 directly under items applies them to the inner
arrays rather than the underlying integer values. This renders the numeric
limits a silent no-op, allowing out-of-bounds reg values to bypass validation.

[ ... ]

> @@ -98,6 +122,21 @@ required:
>  
>  additionalProperties: false
>  
> +allOf:
> +  - $ref: /schemas/i2c/i2c-atr.yaml#
> +
> +  - anyOf:
> +      - oneOf:
> +          - required: [i2c-atr]
> +          - required: [i2c-gate]
> +
> +      - not:
> +          required: [i2c-atr, i2c-gate]

[Severity: Low]
This isn't a bug, but is the oneOf branch needed here?

In JSON Schema, required: [i2c-atr, i2c-gate] evaluates to true only when both
are present. Therefore, the not: required: [i2c-atr, i2c-gate] branch evaluates
to true when zero or one are present, inherently covering all valid cases and
subsuming the oneOf branch entirely.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260807-gmsl2-3_serdes-v15-0-7212e9e5156a@analog.com?part=2