Re: [PATCH v2 01/19] dt-bindings: crypto: add Rambus CryptoManager Hub

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] Commit message and schema discrepancy regarding child node compatible strings.
- [Low] Ambiguous or missing standard DMA properties in schema.
- [Low] Missing programmatic bounds validation for mailbox configuration properties.
--

--- Patch [1]: [PATCH v2 01/19] dt-bindings: crypto: add Rambus CryptoManager Hub ---
commit 4a05b4db54dfefd3f8865951add1c86499b1b7bd
Author: Alex Ousherovitch <aousherovitch@rambus.com>

dt-bindings: crypto: add Rambus CryptoManager Hub

This commit adds a device tree binding schema for the CRI CryptoManager Hub
hardware crypto accelerator. It defines the SoC-level node properties
including registers, interrupts, and mailbox configuration, along with
per-core child nodes.

> Add device tree binding schema for the CRI CryptoManager Hub (CMH)
> hardware crypto accelerator.  The binding covers the parent SoC-level
> node with register region, interrupt, DMA properties, and per-core
> child nodes identified by compatible string and unit address.

[Severity: Medium]
The commit message states that per-core child nodes are identified by a
compatible string and unit address. However, looking at the schema definition
for the child nodes, they strictly set additionalProperties: false and do not
define or allow a compatible property. Will validation fail if a device tree
author adds a compatible string to a child node as described here?

[Severity: Low]
The commit message also mentions that the binding covers DMA properties.
Since the root node sets additionalProperties: false without referencing
standard DMA properties like dmas or dma-coherent, they won't be permitted.
If standard DMA properties are intended, shouldn't they be explicitly allowed
in the schema? If the message is referring to the custom cri,mbx-* properties,
the phrasing might be slightly ambiguous.

> diff --git a/Documentation/devicetree/bindings/crypto/cri,cmh.yaml b/Documentation/devicetree/bindings/crypto/cri,cmh.yaml
> new file mode 100644
> index 0000000000000..db41132e0591d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/crypto/cri,cmh.yaml
[ ... ]
> +  cri,mbx-slots-log2:
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    minItems: 1
> +    maxItems: 64
> +    description:
> +      Per-mailbox slot count as log2.  Valid range 1..15.
> +      Array length must match cri,mbx-instances.
> +      Default is 5 (32 slots).

[Severity: Low]
Does this property need YAML constraints to enforce the documented range?
Without minimum and maximum constraints for the items, invalid device trees
specifying out-of-range values might silently pass schema validation.

> +
> +  cri,mbx-strides-log2:
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    minItems: 1
> +    maxItems: 64
> +    description:
> +      Per-mailbox stride (bytes per slot) as log2.  Valid range 7..10.
> +      Array length must match cri,mbx-instances.
> +      Default is 7 (128 bytes per slot).

[Severity: Low]
Similar to the slots property above, can we add minimum and maximum bounds
to the schema here so that tooling can automatically catch values outside
the 7 to 10 range?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709203037.1884436-1-skrishnamoorthy@rambus.com?part=1