Re: [PATCH v21 2/6] dt-bindings: remoteproc: Add STM32 TEE-controlled rproc binding

Hello Krzysztof,


On 3/19/26 09:06, Krzysztof Kozlowski wrote:
> On Tue, Mar 17, 2026 at 07:03:23PM +0100, Arnaud Pouliquen wrote:
>> Add a Device Tree binding for the STM32 remote processor controlled
>> via a Trusted Application running in OP-TEE.
>> This binding describes the interface and properties required for STM32MP
>> remoteproc instances managed by the TEE rproc service, including a
>> linkage to the TEE backend through the property "rproc-tee-phandle".
>>
>> Signed-off-by: Arnaud Pouliquen <[email protected]>
>> ---
>> V21 updates:
>> - the m4 node is no more declared as a child of the optee-rproc node
>> - "rproc-tee-phandle" property is introduced to reference the optee-rproc
>> ---
>>   .../remoteproc/st,stm32-rproc-tee.yaml        | 108 ++++++++++++++++++
>>   1 file changed, 108 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/remoteproc/st,stm32-rproc-tee.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/remoteproc/st,stm32-rproc-tee.yaml b/Documentation/devicetree/bindings/remoteproc/st,stm32-rproc-tee.yaml
>> new file mode 100644
>> index 000000000000..ca4dd1c8e7b0
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/remoteproc/st,stm32-rproc-tee.yaml
>> @@ -0,0 +1,108 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/remoteproc/st,stm32-rproc-tee.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: STMicroelectronics STM32 remote processor controlled via TEE
>> +
>> +maintainers:
>> +  - Arnaud Pouliquen <[email protected]>
>> +
>> +description: |
>> +  STM32MP remote processor controlled by a Trusted Application
>> +  running in OP-TEE. This node is a child of the TEE remoteproc service
>> +  (UUID 80a4c275-0a47-4905-8285-1486a9771a08) and exposes a remoteproc
>> +  instance managed by the Linux remoteproc core via the TEE rproc service.
>> +
>> +  Firmware loading, authentication and remote processor start/stop are managed
>> +  by the TEE application. The STM32-specific driver handles platform resources
>> +  such as the mailboxes and reserved-memory.
>> +
>> +properties:
>> +  compatible:
>> +    const: st,stm32mp1-m4-tee
> 
> Drop "tee", it suggests that compatible is tied to implementation of FW
> you put there.

The "st,stm32mp1-m4" compatible string already exists in 
drivers/remoteproc/stm32_rproc.c, and "st,stm32mp1-m4-tee" compatible is 
upstreamed in OP-TEE.

Notice that I have also the stm32mp2 SoC to upstream expecting to have 
similar compatible:
- st,stm32mp1-m33
- st,stm32mp2-m33-tee

Depending on the compatible string, the hardware behavior changes.
With the "xxxx-tee" compatible, OP-TEE also manages the isolation of 
remote processor resources (memory, clock reset, peripherals).
Without the "xxxx-tee" compatible, OP-TEE have to ensure that the Linux
has the good access right to manage the remote processor.

For instance if st,stm32mp1-m4-tee is set instead of st,stm32mp1-m4, on
linux side
- only memory regions used for IPC should be declared
- memory regions containing the remote firmware must not be declared as 
not accessible by the Linux ( managed by OP-TEE).
- resets must not be declared ( managed by OP-TEE)

You probably don't remember, as it was a long time ago, but we already 
discussed this point with Rob[1].
[1] https://lkml.org/lkml/2024/1/18/100

Do it still reasonable to you and Rob or should we find an alternative?


Thanks and Regards,

Arnaud

> 
>> +
>> +  reg:
>> +    description: |
> 
> Do not need '|' unless you need to preserve formatting.
> 
>> +      Remote processor identifier used by the TEE service. The <0> value
>> +      in the example denotes a single instance with ID 0.
>> +    maxItems: 1
>> +
>> +  mboxes:
>> +    description: |
>> +      Mailbox channels used for rpmsg/virtio functionality and processor
>> +      shutdown.
>> +    maxItems: 3
>> +
>> +  mbox-names:
>> +    items:
>> +      - const: vq0
>> +      - const: vq1
>> +      - const: shutdown
>> +
>> +  memory-region:
>> +    description: |
>> +      List of phandles to reserved-memory nodes describing the memory layout
>> +      for the interprocessors communication.
> 
> Drop description. You miss maxItems, though.
> 
>> +
>> +  interrupts:
>> +    description: |
> 
> Same comments...
> 
>> +      Optional watchdog / status interrupt line used to detect crashes
>> +      and optionally wake up the system.
>> +    maxItems: 1
>> +
>> +  st,auto-boot:
>> +    type: boolean
>> +    description: |
>> +      If present, the remote processor will be automatically started by
>> +      the remoteproc core at boot.
> 
> That's policy, not DT property.
> 
>> +
>> +  wakeup-source:
>> +    type: boolean
>> +    description: |
>> +      Indicates that the watchdog interrupt can be used as a wakeup source.
>> +
>> +  rproc-tee-phandle:
> 
> Missing vendor prefix, drop phandle. You do not say that "st,auto-boot"
> is "st,auto-boot-boolean"
> 
>> +    description: |
>> +      Phandle to the remote processor backend node and its identifier. This property
>> +      is used to link the TEE remoteproc service to the remote processor instance
>> +      it controls. The value is a phandle reference to the remote processor node,
>> +      followed by a cell specifying the remote processor identifier used by the TEE.
>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> 
> Missing constraints.
> 
> I also do not understand which bus node this is being child, if not
> remote proc.
> 
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - rproc-tee-phandle
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    firmware {
>> +        tee_rproc: optee-rproc {
>> +            compatible = "80a4c275-0a47-4905-8285-1486a9771a08";
>> +        };
>> +    };
> 
> Drop
> 
>> +
>> +    m4: m4@0 {
> 
> Drop unused label.
> 
> Node names should be generic. See also an explanation and list of
> examples (not exhaustive) in DT specification:
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> If you cannot find a name matching your device, please check in kernel
> sources for similar cases or you can grow the spec (via pull request to
> DT spec repo).
> 
> e.g. mcu, because m4 feels like given model (Cortex M4?).
> 
>> +      compatible = "st,stm32mp1-m4-tee";
>> +      reg = <0 0>;
>> +
>> +      mboxes = <&ipcc 0>, <&ipcc 1>, <&ipcc 2>;
>> +      mbox-names = "vq0", "vq1", "shutdown";
>> +
>> +      memory-region = <&vdev0vring0>, <&m_ipc_shm>, <&mcuram2>,
>> +                      <&vdev0vring1>, <&vdev0buffer>, <&retram>;
>> +
>> +      interrupt-parent = <&exti>;
>> +      interrupts = <68 1>;
>> +
>> +      rproc-tee-phandle = <&tee_rproc 0>;
>> +      st,auto-boot;
>> +      wakeup-source;
>> +
>> +      status = "okay";
> 
> Drop
> 
> Best regards,
> Krzysztof
>