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

[Krzysztof Kozlowski <krzk@kernel.org>]

On 22/09/2026 05:47, 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.

Thank you for your patch. There is something to discuss/improve.


> 
> 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.
> 
> Cc: Jassi Brar <jaswinder.singh@linaro.org>
> Cc: Samuel Holland <samuel@sholland.org>
> Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> Cc: devicetree@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-sunxi@lists.linux.dev

Please drop the autogenerated scripts/get_maintainer.pl CC-entries from
commit msg. There is no single need to store automated output of
get_maintainers.pl in the git log. It can be easily re-created at any
given time, thus its presence in the git history is redundant and
obfuscates the log.

If you need it for your own patch management purposes, keep it under the
--- separator.

> Signed-off-by: Tim Michals <tcmichals@gmail.com>
> ---
>  .../mailbox/allwinner,sun55i-a523-msgbox.yaml | 93 +++++++++++++++++++
>  1 file changed, 93 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mailbox/allwinner,sun55i-a523-msgbox.yaml
> 
> 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 000000000000..4b73543c7902
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mailbox/allwinner,sun55i-a523-msgbox.yaml
> @@ -0,0 +1,93 @@
> +# 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:
> +    minItems: 4

You can drop minItems

> +    maxItems: 4
> +    description: |

Do not need '|' unless you need to preserve formatting.

> +      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
> +    description: bus clock

Redundant description, drop.

> +
> +  resets:
> +    maxItems: 1
> +    description: bus reset

Also here

> +
> +  interrupts:
> +    minItems: 1
> +    maxItems: 4

How interrupt could be optional? It either is or is not there.

> +    description: |
> +      One interrupt per processor port. The host port interrupt is required;
> +      remote port interrupts are optional.
> +
> +  '#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




Best regards,
Krzysztof