Re: [PATCH v15 3/8] dt-bindings: i3c: Add NXP P3H2x4x i3c-hub support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] The schema fails to restrict the allowed ports and properties for the 4-port hardware variants (P3H2440/P3H2441), allowing invalid configurations.
- [Medium] The `reg` property constraints for child nodes (`i3c@[0-7]$` and `smbus@[0-7]$`) use an invalid array-level constraint `items: - maximum: 7` which silently bypasses port index validation.
- [Medium] The hub acts as a parent to child bus nodes but does not mandate `#address-cells` and `#size-cells` in its required list.
--

--- Patch [3]: [PATCH v15 3/8] dt-bindings: i3c: Add NXP P3H2x4x i3c-hub support ---
commit 9450f00b16aca24baa4b725459c27c4b2ab2f58c
Author: Aman Kumar Pandey <aman.kumarpandey@nxp.com>

dt-bindings: i3c: Add NXP P3H2x4x i3c-hub support

This commit adds bindings for the NXP P3H2x4x (P3H2440/P3H2441/P3H2840/P3H2841)
multiport I3C hub family. These devices connect to a host via I3C/I2C/SMBus
and allow communication with multiple downstream peripherals.

> diff --git a/Documentation/devicetree/bindings/i3c/nxp,p3h2840.yaml b/Documentation/devicetree/bindings/i3c/nxp,p3h2840.yaml
> new file mode 100644
> index 0000000000000..6b84ccd8ace0c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i3c/nxp,p3h2840.yaml
[ ... ]
> +properties:
> +  compatible:
> +    enum:
> +      - nxp,p3h2440
> +      - nxp,p3h2441

[Severity: Medium]
Should there be an allOf conditional block added to constrain the allowed
ports and properties for the 4-port variants?

The 4-port hardware variants are introduced here but lack conditional blocks
restricting their allowed ports, while the patternProperties later in the
schema unconditionally allows up to 8 ports.

Without an allOf block, device tree configurations that describe nonexistent
ports for the 4-port variants could silently pass schema validation.

> +      - nxp,p3h2840
> +      - nxp,p3h2841
[ ... ]
> +required:
> +  - reg

[Severity: Medium]
Should #address-cells and #size-cells be included in this required list?

Since the hub node acts as a parent bus to child target ports, omitting these
properties allows device trees to define child nodes without proper address
sizing, which might bypass schema validation when they shouldn't.

> +
> +patternProperties:
> +  "^i3c@[0-7]$":
> +    type: object
> +    $ref: /schemas/i3c/i3c.yaml#
> +    unevaluatedProperties: false
> +
> +    properties:
> +      reg:
> +        description:
> +          The I3C HUB Target Port number.
> +        items:
> +          - maximum: 7

[Severity: Medium]
Does this constraint correctly validate the port index?

Because dt-schema parses reg as a matrix (an array of cell arrays), JSON
Schema only applies maximum to numeric types and silently ignores this
constraint on the array element. This could allow an out-of-range port number
to bypass port index validation.

To correctly validate the single cell, it might need to use maxItems: 1 and
nested items.

[ ... ]
> +  "^smbus@[0-7]$":
> +    type: object
> +    $ref: /schemas/i2c/i2c-controller.yaml#
> +    unevaluatedProperties: false
> +
> +    properties:
> +      reg:
> +        description:
> +          The I3C HUB Target Port number.
> +        items:
> +          - maximum: 7

[Severity: Medium]
Does this have the same validation issue as the i3c child node above?

Like the i3c node, this constraint might fail to validate the port index
because JSON Schema ignores maximum on array elements.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260817103844.2142802-1-lakshay.piplani@nxp.com?part=3

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c