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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [Low] dt-bindings: missing closing brace in device tree example
- [Low] dt-bindings: missing schema constraints for memory-region-names
- [Low] dt-bindings: missing schema enforcement for required r_sram regions
- [Low] dt-bindings: unnecessary status property redeclaration
- [Low] dt-bindings: unused arm-gic.h include in example
--

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

dt-bindings: remoteproc: add allwinner sun55i rproc binding
    
Add Device Tree binding schema for the Allwinner XuanTie E906/E907
RISC-V remote processor found on A523, A527, and T527 SoCs.

> diff --git a/Documentation/devicetree/bindings/remoteproc/allwinner,sun55i-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/allwinner,sun55i-rproc.yaml
> new file mode 100644
> index 0000000000000..5d8b929f60b67
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/remoteproc/allwinner,sun55i-rproc.yaml
> @@ -0,0 +1,152 @@

[ ... ]

> +  reg:
> +    minItems: 1
> +    maxItems: 4
> +    description: |
> +      Memory-mapped register regions. The following named regions are
> +      supported (all optional except at least one of r_sram or r_sram1):
> +        "cfg"    - RISC-V core control and boot-vector registers (0x07130000)
> +        "r_sram" - Dedicated MCU SRAM Space 0 (0x07280000 / 0x07200000; 256-512 KB)
> +        "r_sram1"- Switchable MCU SRAM Space 1 / SRAMA3_2 (0x072c0000 / 0x07280000; 256-512 KB)
> +        "remap"  - Hardware Remap Control Register (offset 0x364)
> +
> +  reg-names:
> +    minItems: 1
> +    maxItems: 4
> +    items:
> +      enum: [cfg, r_sram, r_sram1, remap]

[Severity: Low]
The description mentions a hardware requirement that at least one of "r_sram"
or "r_sram1" must be present. Should this constraint be enforced in the schema
(for example, using an anyOf block with contains on reg-names) so that a
device tree omitting both won't silently pass validation?

[ ... ]

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

[Severity: Low]
Does this property need JSON schema constraints to enforce the allowed string
values? Without items and enum definitions, the schema allows any string
value or arbitrary array lengths to pass validation silently.

[ ... ]

> +  interrupt-names:
> +    items:
> +      - const: crash
> +
> +  status: true

[Severity: Low]
Is this redeclaration of the status property needed? Core device tree schemas
already declare the status property globally, so redeclaring it here without
strict constraints seems unnecessary.

[ ... ]

> +examples:
> +  - |
> +    #include <dt-bindings/clock/sun55i-a523-ccu.h>
> +    #include <dt-bindings/reset/sun55i-a523-ccu.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>

[Severity: Low]
Is this include needed? There are no interrupts defined in the example node,
so the macros from arm-gic.h appear to be unused dead code.

> +
> +    remoteproc@7130000 {
> +        compatible = "allwinner,sun55i-a523-rproc";
> +        reg = <0x07130000 0x1000>,
> +              <0x07280000 0x40000>,
> +              <0x072c0000 0x40000>,
> +              <0x07010364 0x4>;
> +        reg-names = "cfg", "r_sram", "r_sram1", "remap";
> +        clocks = <&mcu_ccu CLK_BUS_MCU_RISCV_CFG>,
> +                 <&mcu_ccu CLK_MCU_RISCV>;
> +        clock-names = "bus", "core";
> +        resets = <&mcu_ccu RST_BUS_MCU_RISCV_CFG>,
> +                 <&mcu_ccu RST_BUS_MCU_RISCV_CORE>;
> +        reset-names = "cfg", "core";
> +        mboxes = <&msgbox 0>, <&msgbox 1>;
> +        mbox-names = "rx", "tx";
> +        firmware-name = "testBasic.elf";

[Severity: Low]
Is there a missing closing brace and semicolon at the end of this example
node? Without it, dtc may report a syntax error when the dt_binding_check
build target compiles the example.

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