Re: [PATCH v12 12/22] dt-bindings: media: i2c: add MAX9296A, MAX96716A, MAX96792A

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 9 potential issue(s) to consider:
- [Medium] The i2c-alias-pool property is restricted to a maximum of 2 items, breaking support for standard camera topologies.
- [Medium] Validation constraints for endpoint nodes can be bypassed because they are defined as literal properties rather than a pattern.
- [Medium] Missing if-then conditional restrictions for the single-link MAX96716A variant.
- [Medium] Missing interrupts property for the INTB pin.
- [Medium] Missing gpio-controller capabilities in the schema properties.
- [Low] The i2c-atr node omits #address-cells and #size-cells from its required list.
- [Low] The i2c@[0-1] child nodes omit the reg property from their required list.
- [Low] Invalid JSON Schema constraints applied to the reg property inside i2c@[0-1].
- [Low] Incorrect use of additionalProperties: false with an allOf $ref.
--

commit 876d721dbaadb4f3d383ec8e1a846d525f5e14ea
Author: Cosmin Tanislav <demonsingur@gmail.com>

dt-bindings: media: i2c: add MAX9296A, MAX96716A, MAX96792A

This commit introduces the device tree bindings for the Maxim MAX9296A,
MAX96716A, and MAX96792A GMSL2 to CSI-2 deserializers. It defines properties
for ports, i2c alias pool, i2c-atr, and GPIO powerdown.

> diff --git a/Documentation/devicetree/bindings/media/i2c/maxim,max9296a.yaml b/Documentation/devicetree/bindings/media/i2c/maxim,max9296a.yaml
> new file mode 100644
> index 0000000000000..de6bfcec5dc63
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/maxim,max9296a.yaml
> @@ -0,0 +1,242 @@

[ ... ]

> +properties:
> +  compatible:
> +    enum:
> +      - maxim,max9296a
> +      - maxim,max96716a
> +      - maxim,max96792a
> +

[Severity: Medium]
GMSL deserializers such as the MAX9296A typically feature an interrupt pin
(INTB) used to signal events to the host SoC. Because the schema strictly
enforces additionalProperties: false at the top level and omits the
interrupts property, will valid device tree nodes that specify an interrupts
property to describe this hardware connection be erroneously rejected by the
schema validator?

[Severity: Medium]
Maxim GMSL2 deserializers, including the MAX9296A, incorporate GPIO
capabilities. The schema omits gpio-controller and #gpio-cells from the
properties block. Will device trees attempting to use the deserializer as a
GPIO provider fail validation? Should these properties be allowed by the
schema to accurately reflect the hardware's capabilities?

> +  reg:
> +    maxItems: 1
> +
> +  powerdown-gpios:
> +    maxItems: 1
> +    description: Specifier for the GPIO connected to the PWDNB pin.
> +
> +  port0-poc-supply:
> +    description: Regulator providing Power over Coax for GMSL port 0
> +
> +  port1-poc-supply:
> +    description: Regulator providing Power over Coax for GMSL port 1
> +
> +  i2c-alias-pool:
> +    maxItems: 2

[Severity: Medium]
A dual-link GMSL deserializer like the MAX9296A connects to two serializers,
each potentially having multiple sensors or other I2C clients. The Address
Translator (ATR) needs an alias for each remote client. A basic configuration
with 2 serializers and 2 sensors requires at least 4 aliases. Does
restricting i2c-alias-pool to maxItems: 2 limit the system to a maximum of
two remote I2C clients, causing valid hardware configurations to incorrectly
fail schema validation?

> +
> +  i2c-atr:
> +    type: object
> +    additionalProperties: false
> +
> +    properties:
> +      '#address-cells':
> +        const: 1
> +
> +      '#size-cells':
> +        const: 0
> +
> +    patternProperties:
> +      '^i2c@[0-1]$':
> +        $ref: /schemas/i2c/i2c-controller.yaml#
> +        unevaluatedProperties: false
> +        properties:
> +          reg:
> +            items:
> +              minimum: 0
> +              maximum: 1

[Severity: Low]
The i2c-atr object defines child nodes with unit addresses (^i2c@[0-1]$).
According to Device Tree specifications, any node with unit-addressed
children must define #address-cells and #size-cells. While these properties
are defined in the schema block, they are not marked as required. Will
structurally invalid device trees missing these properties incorrectly pass
schema validation?

[Severity: Low]
The core DT schema defines reg as a matrix (an array of arrays). Applying
items: { minimum: 0, maximum: 1 } directly under reg applies numeric bounds
to the inner arrays themselves rather than their contents. Should the inner
array bounds be formatted correctly, or should we simply rely on the unit
address regex and standard dtc checks to avoid a JSON schema type mismatch?

[Severity: Low]
The i2c@[0-1] child nodes contain unit addresses in their names. Device tree
specifications require nodes with a unit address to have a matching reg
property. The schema defines the reg property for these nodes but fails to
list it in a required array. Can configurations omitting the reg property
incorrectly pass schema validation?

> +
> +  ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +
> +    patternProperties:
> +      '^port@[0-1]$':
> +        $ref: /schemas/graph.yaml#/properties/port
> +        description: GMSL Input ports 0-1
> +
> +      '^port@[2-3]$':
> +        $ref: /schemas/graph.yaml#/$defs/port-base
> +        unevaluatedProperties: false
> +        description: CSI-2 Output ports 0-1
> +        properties:
> +          endpoint:

[Severity: Medium]
Under port@[2-3], constraints like data-lanes are applied using properties:
endpoint:. However, the standard port-base schema allows endpoints with unit
addresses, such as endpoint@0. If a device tree uses endpoint@0, it will not
match the literal endpoint key, completely bypassing all defined constraints.
Should the schema use patternProperties: "^endpoint(@[0-9a-f]+)?$" to
ensure constraints apply to any valid endpoint node?

[ ... ]

> +required:
> +  - compatible
> +  - reg
> +  - ports
> +
> +additionalProperties: false

[Severity: Low]
The schema includes an allOf block referencing /schemas/i2c/i2c-atr.yaml#,
but uses additionalProperties: false at the top level. According to dt-schema
writing guidelines, unevaluatedProperties: false must be used when importing
other schemas via allOf. Does using additionalProperties restrict validation
only to the explicitly listed local properties, breaking inheritance?

> +
> +allOf:
> +  - $ref: /schemas/i2c/i2c-atr.yaml#

[Severity: Medium]
The MAX96716A is a single-link device, unlike the dual-link variants.
However, the schema unconditionally allows port@1, port1-poc-supply, and the
i2c@1 child node for all compatible strings. Without an if-then block
restricting these secondary link properties for maxim,max96716a, could
incomplete or incorrect device trees defining a second link for the
single-link hardware improperly pass schema validation?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260521-gmsl2-3_serdes-v12-0-b26d92931196@analog.com?part=12