From: Xing Loong <xing.xl.loong@gmail.com>
To: Krzysztof Kozlowski <krzk@kernel.org>
Cc: Jens Wiklander <jenswi@kernel.org>,
Krzysztof Kozlowski <krzk+dt@kernel.org>,
Conor Dooley <conor+dt@kernel.org>, Rob Herring <robh@kernel.org>,
Sumit Garg <sumit.garg@kernel.org>,
op-tee@lists.trustedfirmware.org, devicetree@vger.kernel.org,
linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/3] dt-bindings: firmware: add mbedtee,rpc binding
Date: Fri, 3 Jul 2026 15:16:29 +0800 [thread overview]
Message-ID: <akdhzUQWPsUS69P5@ubuntu24> (raw)
In-Reply-To: <70068ae8-ede1-41d2-899b-98ed11e19953@kernel.org>
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 <xing.xl.loong@gmail.com>
> > ---
> > .../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 <xing.xl.loong@gmail.com>
> > +
> > +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
next prev parent reply other threads:[~2026-07-03 7:16 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-01 13:25 [PATCH 0/3] tee: add MbedTEE driver Xing Loong
2026-07-01 13:25 ` [PATCH 1/3] dt-bindings: vendor-prefixes: add mbedtee Xing Loong
2026-07-01 13:25 ` [PATCH 2/3] dt-bindings: firmware: add mbedtee,rpc binding Xing Loong
2026-07-01 14:05 ` Krzysztof Kozlowski
2026-07-03 7:16 ` Xing Loong [this message]
2026-07-01 16:39 ` Rob Herring (Arm)
2026-07-01 13:25 ` [PATCH 3/3] tee: add MbedTEE driver Xing Loong
2026-07-02 15:11 ` [PATCH v2 0/3] " Xing Loong
2026-07-02 15:11 ` [PATCH v2 1/3] dt-bindings: vendor-prefixes: add mbedtee Xing Loong
2026-07-02 15:11 ` [PATCH v2 2/3] dt-bindings: firmware: add mbedtee,tee binding Xing Loong
2026-07-02 15:27 ` Krzysztof Kozlowski
2026-07-02 15:11 ` [PATCH v2 3/3] tee: add MbedTEE driver Xing Loong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=akdhzUQWPsUS69P5@ubuntu24 \
--to=xing.xl.loong@gmail.com \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=jenswi@kernel.org \
--cc=krzk+dt@kernel.org \
--cc=krzk@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=op-tee@lists.trustedfirmware.org \
--cc=robh@kernel.org \
--cc=sumit.garg@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox