Re: [PATCH 2/3] dt-bindings: firmware: add mbedtee,rpc binding

On 01/07/2026 16:05, Krzysztof Kozlowski wrote:
> On 01/07/2026 15:25, Xing Loong wrote:
> > Add YAML devicetree binding for the MbedTEE Trusted Execution
> 
> Drop YAML, there is no such thing as YAML binding.

Will drop.

> > Environment driver.
> 
> We don't take bindings for drivers but for hardware or firmware. Please
> describe these instead.

Will rewrite the description to describe what MbedTEE is as
firmware, not the driver or the binding.

> > The binding covers two platform configurations:
> >   - ARM/AArch64 (TrustZone, SMC): two reserved-memory regions
> >     (rpc-t2r-ring and rpc-t2r-shm) plus a GIC SPI edge interrupt
> >     for TEE-to-REE notifications.
> >   - RISC-V (IMSIC): three reserved-memory regions, adding
> >     rpc-r2t-ring for REE-to-TEE command submissions; no interrupts
> >     property (T2R notifications use IMSIC MSI allocated at runtime).
> > 
> > Signed-off-by: Xing Loong <[email protected]>
> > ---
> >  .../bindings/firmware/mbedtee,rpc.yaml        | 221 ++++++++++++++++++
> >  1 file changed, 221 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/firmware/mbedtee,rpc.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/firmware/mbedtee,rpc.yaml b/Documentation/devicetree/bindings/firmware/mbedtee,rpc.yaml
> > new file mode 100644
> > index 0000000..08ae255
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/firmware/mbedtee,rpc.yaml
> > @@ -0,0 +1,221 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/firmware/mbedtee,rpc.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: MbedTEE Trusted Execution Environment
> > +
> > +maintainers:
> > +  - Xing Loong <[email protected]>
> > +
> > +description: |
> > +  MbedTEE is a Trusted Execution Environment for embedded systems.
> > +  This binding describes the shared-memory regions used for RPC
> 
> Describe firmware, not the binding. It's redundant to say what the
> binding is about, just say what is the hardware.

Will rewrite, removing "driver", "YAML" and "This binding
describes" wording throughout.

> > +  communication between the Linux REE driver and MbedTEE OS.
> > +
> > +  The REE and TEE CPUs sharing the RPC memory must be in a
> > +  hardware-coherent domain (same CPU cluster, coherent caches).
> > +
> > +  Two or three reserved-memory regions are required:
> > +
> > +    rpc-t2r-ring  ring buffer for TEE-to-REE notifications (all platforms)
> > +    rpc-t2r-shm   shared memory for TEE-to-REE RPC payloads (all platforms)
> > +    rpc-r2t-ring  ring buffer for REE-to-TEE command submissions (RISC-V only)
> > +
> > +  On ARM/AArch64 the transport uses SMC calls; TEE-to-REE
> > +  notifications use a GIC SPI edge interrupt.
> > +
> > +  On RISC-V the TEE notifies the REE via IMSIC MSI; the REE submits
> > +  commands via shared-memory rpc-r2t-ring that the TEE polls. No
> > +  REE-to-TEE interrupt is used. No SBI ecall is involved.
> > +
> > +properties:
> > +  $nodename:
> > +    const: mbedtee
> 
> Drop, why would it be relevant?

Will drop.

> > +
> > +  compatible:
> > +    const: mbedtee,rpc
> 
> Feels way too generic. First, Google results on mbedtee are basically
> non-existing, so what sort of company is that?
> 
> Second, rpc is just not specific enough. Please carefully read writing
> bindings doc.

Will rename to "mbedtee,tee". mbedtee is an open-source TEE
project (https://github.com/mbedtee), not a company. Patch 1/3 adds
the corresponding "mbedtee" entry to vendor-prefixes.yaml.

> > +
> > +  interrupts:
> > +    description:
> > +      GIC interrupt used by the TEE to notify the REE of pending RPC
> > +      responses (ARM/AArch64 only). Not present on RISC-V platforms which
> > +      use IMSIC platform MSI interrupts allocated dynamically at runtime.
> 
> Please read writing bindings doc.

Will drop the description.  Will add maxItems: 1 instead.

> > +
> > +  msi-parent:
> > +    maxItems: 1
> > +    description:
> > +      IMSIC MSI controller used by the Linux driver to allocate the
> 
> Again drivers...

Will drop the description.  Will keep only maxItems: 1.

> > +      TEE-to-REE notification interrupt on RISC-V platforms. Not present on
> > +      ARM/AArch64 platforms, which use the interrupts property.
> > +
> > +  memory-region:
> > +    minItems: 2
> > +    maxItems: 3
> > +    description:
> > +      References to reserved-memory regions for REE<->TEE communication.
> > +      Entries must match memory-region-names order.
> 
> Obvious. Please do not come with your own style of bindings.

Will drop the description.  Understood, will follow the standard
style used by existing bindings.

> > +
> > +  memory-region-names:
> > +    minItems: 2
> > +    maxItems: 3
> 
> Why is this flexible?

Will constrain it via the allOf: if/then branches (ARM: fixed at 2,
RISC-V: fixed at 3).

> > +    items:
> > +      enum:
> > +        - rpc-t2r-ring
> 
> rpc is redundant, drop

Will drop the "rpc-" prefix.  Will use t2r-ring, t2r-shm, r2t-ring.

> > +        - rpc-t2r-shm
> > +        - rpc-r2t-ring
> > +
> > +required:
> > +  - compatible
> > +
> > +allOf:
> > +  - if:
> > +      required:
> > +        - interrupts
> > +    then:
> > +      required:
> > +        - interrupts
> > +        - memory-region
> > +        - memory-region-names
> > +      properties:
> > +        msi-parent: false
> > +        memory-region:
> > +          minItems: 2
> > +          maxItems: 2
> > +        memory-region-names:
> > +          items:
> > +            - const: rpc-t2r-ring
> > +            - const: rpc-t2r-shm
> > +    else:
> > +      required:
> > +        - msi-parent
> > +        - memory-region
> > +        - memory-region-names
> 
> So memory-region is always required?

Yes.  Will move memory-region and memory-region-names to the
top-level required: list to make this explicit.  The interrupts and
msi-parent properties will also be made mutually exclusive per
platform (msi-parent: false on ARM, interrupts: false on RISC-V).

> > +      properties:
> > +        memory-region:
> > +          minItems: 3
> > +          maxItems: 3
> > +        memory-region-names:
> > +          items:
> > +            - const: rpc-t2r-ring
> > +            - const: rpc-t2r-shm
> > +            - const: rpc-r2t-ring
> 
> Your top level schema said that. You only need minItems.

Will drop the redundant constraints from the else branch.

> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    /* ARM TrustZone (SMC) */
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    / {
> > +      #address-cells = <2>;
> > +      #size-cells = <2>;
> > +
> > +      gic: interrupt-controller@2f000000 {
> > +        compatible = "arm,gic-v3";
> > +        reg = <0 0x2f000000 0 0x10000>,
> > +              <0 0x2f100000 0 0x200000>;
> > +        interrupt-controller;
> > +        #interrupt-cells = <3>;
> > +      };
> > +
> > +      reserved-memory {
> > +        #address-cells = <2>;
> > +        #size-cells = <2>;
> > +        ranges;
> > +
> > +        mbedtee_t2r_ring: rpc-t2r-ring@85f10000 {
> > +          reg = <0 0x85f10000 0 0x20000>;
> > +          no-map;
> > +        };
> > +
> > +        mbedtee_t2r_shm: rpc-t2r-shm@85f30000 {
> > +          reg = <0 0x85f30000 0 0x40000>;
> > +          no-map;
> > +        };
> > +      };
> 
> None of the above is relevant, drop.

Will drop all platform infrastructure nodes.  Will follow the
minimal example pattern seen in linaro,optee-tz.yaml.

> > +
> > +      firmware {
> > +        mbedtee {
> > +          compatible = "mbedtee,rpc";
> > +          interrupt-parent = <&gic>;
> > +          interrupts = <GIC_SPI 72 IRQ_TYPE_EDGE_RISING>;
> > +          memory-region = <&mbedtee_t2r_ring>, <&mbedtee_t2r_shm>;
> > +          memory-region-names = "rpc-t2r-ring", "rpc-t2r-shm";
> > +        };
> > +      };
> > +    };
> > +
> 
> Best regards,
> Krzysztof

Thanks for the thorough review.  Will send v3 after incorporating
all of the above.

Best regards,
Xing Loong