[v2,1/7] dt-bindings: mailbox: add Allwinner sun55i msgbox schema
Commit Message
Add Device Tree binding schema for the Allwinner 4-port hardware
Message Box controller found on sun55i (A523, A527, T527) and
sun60i (A733) SoCs.
The message box connects the ARM Cortex-A55 host cluster to the
HiFi4 Audio DSP, Power Management Unit (CPUS), and XuanTie RISC-V
co-processor across 12 logical channels with 8-entry hardware FIFOs.
Signed-off-by: Tim Michals <tcmichals@gmail.com>
---
.../mailbox/allwinner,sun55i-a523-msgbox.yaml | 102 ++++++++++++++++++
1 file changed, 102 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mailbox/allwinner,sun55i-a523-msgbox.yaml
Comments
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?
On Sat, Sep 26, 2026 at 07:20:10PM -0500, Tim Michals wrote:
> Add Device Tree binding schema for the Allwinner 4-port hardware
> Message Box controller found on sun55i (A523, A527, T527) and
> sun60i (A733) SoCs.
>
> The message box connects the ARM Cortex-A55 host cluster to the
> HiFi4 Audio DSP, Power Management Unit (CPUS), and XuanTie RISC-V
> co-processor across 12 logical channels with 8-entry hardware FIFOs.
>
> Signed-off-by: Tim Michals <tcmichals@gmail.com>
> ---
> .../mailbox/allwinner,sun55i-a523-msgbox.yaml | 102 ++++++++++++++++++
> 1 file changed, 102 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mailbox/allwinner,sun55i-a523-msgbox.yaml
>
Do not attach (thread) your patchsets to some other threads (unrelated
or older versions). This buries them deep in the mailbox and might
interfere with applying entire sets. See also:
https://elixir.bootlin.com/linux/v6.16-rc2/source/Documentation/process/submitting-patches.rst#L830
...
> + 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.
No, this has to be constrained/specific. See writing bindings and any
existing examples. Also, just list the interrupts with minItems, instead
of free form text.
Best regards,
Krzysztof
new file mode 100644
@@ -0,0 +1,102 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mailbox/allwinner,sun55i-a523-msgbox.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Allwinner sun55i 4-Port Message Box
+
+maintainers:
+ - Tim Michals <tcmichals@gmail.com>
+ - Samuel Holland <samuel@sholland.org>
+
+description:
+ The hardware message box on sun55i (A523, A527, T527) and sun60i (A733)
+ SoCs is a 4-port multi-processor mailbox controller connecting the ARM
+ Cortex-A55 host cluster, HiFi4 Audio DSP, Power Management Unit (CPUS),
+ and XuanTie RISC-V co-processor.
+
+ It features independent 256-byte register banks for each processor port,
+ 12 logical channels on the host (4 per remote processor), and 8-entry
+ hardware FIFOs with level interrupts.
+
+properties:
+ compatible:
+ enum:
+ - allwinner,sun55i-a523-msgbox
+ - allwinner,sun55i-t527-msgbox
+ - allwinner,sun60i-a733-msgbox
+
+ reg:
+ maxItems: 4
+ description:
+ Register banks for each of the four processor ports. Each port has
+ an independent 256-byte register bank.
+
+ reg-names:
+ items:
+ - const: arm
+ - const: dsp
+ - const: cpus
+ - const: rv
+
+ clocks:
+ maxItems: 1
+
+ resets:
+ maxItems: 1
+
+ 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.
+
+ interrupt-names:
+ minItems: 1
+ maxItems: 4
+ items:
+ enum:
+ - arm
+ - dsp
+ - cpus
+ - rv
+
+ '#mbox-cells':
+ const: 1
+ description: "channel number (0-11: 0-3 CPUS, 4-7 DSP, 8-11 RISC-V)"
+
+required:
+ - compatible
+ - reg
+ - reg-names
+ - clocks
+ - resets
+ - interrupts
+ - '#mbox-cells'
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+ mailbox@3003000 {
+ compatible = "allwinner,sun55i-a523-msgbox";
+ reg = <0x03003000 0x1000>,
+ <0x07094000 0x1000>,
+ <0x07120000 0x1000>,
+ <0x07136000 0x1000>;
+ reg-names = "arm", "dsp", "cpus", "rv";
+ clocks = <&ccu 120>;
+ resets = <&ccu 45>;
+ interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 181 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "arm", "dsp", "cpus", "rv";
+ #mbox-cells = <1>;
+ };