Re: [PATCH v16 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:
- [High] Incorrect usage of `items` inside `reg` constraints for the `i2c@[01]` nodes, breaking validation.
- [Medium] The `i2c-atr` node fails to require `#address-cells` and `#size-cells`.
- [Low] The `anyOf` schema block for mutually exclusive properties is logically redundant and unnecessarily complex.
--

commit 1d618fec079831fd8bd91f81cfc0fb4aaa69293f
Author: Cosmin Tanislav <demonsingur@gmail.com>

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

This commit adds I2C Address Translator (ATR) support to the MAX96717 
bindings. It introduces new properties for `i2c-atr` and `i2c-alias-pool` 
while maintaining compatibility with the existing `i2c-gate` approach.

> 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]
Does this node need to mandate `#address-cells` and `#size-cells` in a
`required` block?

Since the `i2c-atr` subnode expects addressable children (`^i2c@[01]$`),
this typically mandates the presence of `#address-cells` and `#size-cells`
on the parent node. Although the schema restricts their values, omitting
them from the `required` list could allow structurally invalid device tree
nodes missing these properties to pass validation silently.

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

[Severity: High]
Are these `minimum` and `maximum` constraints placed correctly?

In `dt-schema`, the `reg` property is parsed as a `uint32-matrix` (an array
of tuples). By nesting `minimum` and `maximum` inside `items`, the schema
attempts to apply numeric constraints to the inner arrays (the tuples)
rather than their scalar values. In JSON Schema, numeric constraints applied
to arrays are silently ignored, which means invalid `reg` values like
`reg = <5>` will silently pass validation.

[ ... ]

> +allOf:
> +  - $ref: /schemas/i2c/i2c-atr.yaml#
> +
> +  - anyOf:
> +      - oneOf:
> +          - required: [i2c-atr]
> +          - required: [i2c-gate]
> +
> +      - not:
> +          required: [i2c-atr, i2c-gate]

[Severity: Low]
Is this `oneOf` branch logically redundant?

The `not` branch evaluates to true whenever at most one of the properties is
present, which entirely subsumes the `oneOf` branch (which evaluates to true
when exactly one is present). Because the `not` branch covers valid
configurations on its own, the `oneOf` branch appears to be unused and could
unnecessarily complicate the schema.

> +
> +dependentRequired:
> +  i2c-atr: [i2c-alias-pool]
> +  i2c-alias-pool: [i2c-atr]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831-gmsl2-3_serdes-v16-0-4fa6e08bb514@analog.com?part=2