Re: [PATCH 4/7] dt-bindings: remoteproc: add allwinner sun55i rproc binding

[Krzysztof Kozlowski <krzk@kernel.org>]

On 22/09/2026 05:47, Tim Michals wrote:
> Add Device Tree binding schema for the Allwinner XuanTie E906/E907
> RISC-V remote processor found on A523, A527, and T527 SoCs.
> 

A nit, subject: drop second/last, redundant "binding". The "dt-bindings"
prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v7.1-rc7/source/Documentation/devicetree/bindings/submitting-patches.rst#L23

...

> +  resets:
> +    minItems: 1
> +    maxItems: 4
> +    description: |
> +      CCU reset lines. Typical resets are "cfg" (configuration block),
> +      "core" (CPU core), "sram" (SRAM interconnect), and "msgbox" (mailbox interconnect).
> +
> +  reset-names:
> +    minItems: 1
> +    maxItems: 4
> +    items:
> +      enum: [cfg, core, sram, msgbox]

Nope, this cannot be flexible. Please open existing code to see how this
is done.

Same comments everywhere else. I don't get why your previous patch had
it correct and here you decided to implement it completely different
style which almost does not exist in the kernel.

> +
> +  mboxes:
> +    minItems: 2

Drpo

> +    maxItems: 2
> +    description:
> +      Exactly two mailbox channels from the Allwinner CPUX_MSGBOX controller —
> +      one receive channel (RISC-V-to-ARM) and one transmit channel
> +      (ARM-to-RISC-V) — used for VirtIO RPMsg kick notifications.
> +
> +  mbox-names:
> +    items:
> +      - const: rx
> +      - const: tx
> +
> +  firmware-name:
> +    $ref: /schemas/types.yaml#/definitions/string

Drop. Instead maxItems. See existing code.

> +    description:
> +      Name of the ELF firmware image to load from /lib/firmware/.
> +      Defaults to "riscv-firmware.elf" if not specified.
> +
> +  memory-region:
> +    description:
> +      Optional phandle list of reserved memory regions for VirtIO vring
> +      buffers or DDR carveouts. Region names "vram" and "dram" are
> +      recognized; "trace" selects the RemoteProc trace buffer carveout.

List the items instead.

> +
> +  memory-region-names:
> +    description:
> +      Names corresponding to the memory-region phandle list entries.
> +      Recognized values are "vram", "dram", and "trace".

List the items instead.

> +
> +  interrupts:
> +    maxItems: 1
> +    description:
> +      Optional hardware crash-notification interrupt. When present the
> +      driver calls rproc_report_crash() on assertion.
> +
> +  interrupt-names:
> +    items:
> +      - const: crash
> +
> +  status: true
> +

Drop


Best regards,
Krzysztof