Re: [PATCH v2 1/7] dt-bindings: mailbox: add Allwinner sun55i msgbox schema

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] dt-bindings: incomplete schema constraints for interrupt-names
- [Medium] dt-bindings: A527 SoC compatible string missing from schema
- [Low] dt-bindings: register bank size mismatch between description and example
--

commit a5a15af0e6145674274afe29234b0c38ce4cb1b7
Author: Tim Michals <tcmichals@gmail.com>

    dt-bindings: mailbox: add Allwinner sun55i msgbox schema

This commit adds a Device Tree binding schema for the Allwinner 4-port hardware
Message Box controller found on sun55i (A523, A527, T527) and sun60i (A733)
SoCs. It defines the constraints for properties like registers, clocks, resets,
and interrupts.

> diff --git a/Documentation/devicetree/bindings/mailbox/allwinner,sun55i-a523-msgbox.yaml b/Documentation/devicetree/bindings/mailbox/allwinner,sun55i-a523-msgbox.yaml
> new file mode 100644
> index 0000000000000..6b0c7ff70c9fb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mailbox/allwinner,sun55i-a523-msgbox.yaml
> @@ -0,0 +1,102 @@
[ ... ]
> +properties:
> +  compatible:
> +    enum:
> +      - allwinner,sun55i-a523-msgbox
> +      - allwinner,sun55i-t527-msgbox
> +      - allwinner,sun60i-a733-msgbox

[Severity: Medium]
The commit message explicitly claims to add support for the Message Box
controller on "sun55i (A523, A527, T527)". The compatible enum provides
strings for A523, T527, and A733, but entirely omits an entry for A527.

Device trees attempting to declare A527 support will fail validation or
must misleadingly use the A523 string without backing documentation.

Should "allwinner,sun55i-a527-msgbox" be included in this enum?

> +
> +  reg:
> +    maxItems: 4
> +    description:
> +      Register banks for each of the four processor ports. Each port has
> +      an independent 256-byte register bank.

[Severity: Low]
Does this description accurately reflect the register bank size?

The text states each port has a 256-byte register bank, but the example
later in the schema maps the hardware's 4KB-aligned register banks (0x1000
bytes):

        reg = <0x03003000 0x1000>,
              <0x07094000 0x1000>,
              <0x07120000 0x1000>,
              <0x07136000 0x1000>;

This creates a documentation mismatch. Should the description be updated
to match the 4K page-aligned size used in practice?

[ ... ]
> +  interrupts:
> +    minItems: 1
> +    maxItems: 4
> +    description:
> +      One interrupt per processor port in port order (arm, dsp, cpus, rv).
> +      The ARM host port interrupt is required; remote port interrupts are
> +      optional. Use interrupt-names to identify which ports are present when
> +      providing a partial list.
[ ... ]
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +  - clocks
> +  - resets
> +  - interrupts
> +  - '#mbox-cells'

[Severity: Medium]
The schema description for interrupts explicitly states that interrupt-names
must be used to identify ports when providing a partial list of optional
interrupts.

However, interrupt-names is not marked as a required property here. As a
result, device tree nodes with ambiguous, partial interrupt lists can
silently pass validation, leaving dependent software unable to reliably
map the provided interrupts to their respective hardware ports.

Could this be enforced in the schema constraints?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260927002021.797069-1-tcmichals@gmail.com?part=1