Hi Arnaud,
First of all apologies for such a late review comment as previously I wasn't CCed or involved in the review of this patch-set. In case any of my following comments have been discussed in the past then feel free to point me at relevant discussions.
On Wed, Jun 25, 2025 at 11:40:26AM +0200, Arnaud Pouliquen wrote:
The "st,stm32mp1-m4-tee" compatible is utilized in a system configuration where the Cortex-M4 firmware is loaded by the Trusted Execution Environment (TEE).
Having a DT based compatible for a TEE service to me just feels like it is redundant here. I can see you have also used a TEE bus based device too but that is not being properly used. I know subsystems like remoteproc, SCMI and others heavily rely on DT to hardcode properties of system firmware which are rather better to be discovered dynamically.
So I have an open question for you and the remoteproc subsystem maintainers being:
Is it feasible to rather leverage the benefits of a fully discoverable TEE bus rather than relying on platform bus/ DT to hardcode firmware properties?
For instance, this compatible is used in both the Linux and OP-TEE device trees:
- In OP-TEE, a node is defined in the device tree with the "st,stm32mp1-m4-tee" compatible to support signed remoteproc firmware. Based on DT properties, the OP-TEE remoteproc framework is initiated to expose a trusted application service to authenticate and load the remote processor firmware provided by the Linux remoteproc framework, as well as to start and stop the remote processor.
- In Linux, when the compatibility is set, the Cortex-M resets should not be declared in the device tree. In such a configuration, the reset is managed by the OP-TEE remoteproc driver and is no longer accessible from the Linux kernel.
Associated with this new compatible, add the "st,proc-id" property to identify the remote processor. This ID is used to define a unique ID, common between Linux, U-Boot, and OP-TEE, to identify a coprocessor.
This "st,proc-id" is just one such property which can rather be directly probed from the TEE/OP-TEE service rather than hardcoding it in DT here. I think the same will apply to other properties as well.
-Sumit
This ID will be used in requests to the OP-TEE remoteproc Trusted Application to specify the remote processor.
Signed-off-by: Arnaud Pouliquen arnaud.pouliquen@foss.st.com Reviewed-by: Rob Herring (Arm) robh@kernel.org
.../bindings/remoteproc/st,stm32-rproc.yaml | 58 ++++++++++++++++--- 1 file changed, 50 insertions(+), 8 deletions(-)
diff --git a/Documentation/devicetree/bindings/remoteproc/st,stm32-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/st,stm32-rproc.yaml index 843679c557e7..58da07e536fc 100644 --- a/Documentation/devicetree/bindings/remoteproc/st,stm32-rproc.yaml +++ b/Documentation/devicetree/bindings/remoteproc/st,stm32-rproc.yaml @@ -16,7 +16,12 @@ maintainers: properties: compatible:
- const: st,stm32mp1-m4
- enum:
- st,stm32mp1-m4- st,stm32mp1-m4-tee- description:
Use "st,stm32mp1-m4" for the Cortex-M4 coprocessor management by non-secure contextUse "st,stm32mp1-m4-tee" for the Cortex-M4 coprocessor management by secure contextreg: description: @@ -43,6 +48,10 @@ properties: - description: The offset of the hold boot setting register - description: The field mask of the hold boot
- st,proc-id:
- description: remote processor identifier
- $ref: /schemas/types.yaml#/definitions/uint32
- st,syscfg-tz: deprecated: true description:
@@ -146,21 +155,43 @@ properties: required:
- compatible
- reg
- resets
allOf:
- if: properties:
reset-names:not:contains:const: hold_boot
compatible:contains: then:const: st,stm32mp1-m4if:properties:reset-names:not:contains:const: hold_bootthen:required:- st,syscfg-holdbootelse:properties:st,syscfg-holdboot: falserequired:- reset-names required:
- st,syscfg-holdboot- else:
- resets
- if:
properties:compatible:contains:const: st,stm32mp1-m4-tee- then: properties: st,syscfg-holdboot: false
reset-names: falseresets: falserequired:- st,proc-idadditionalProperties: false @@ -192,5 +223,16 @@ examples: st,syscfg-rsc-tbl = <&tamp 0x144 0xFFFFFFFF>; st,syscfg-m4-state = <&tamp 0x148 0xFFFFFFFF>; };
- |
- #include <dt-bindings/reset/stm32mp1-resets.h>
- m4@10000000 {
compatible = "st,stm32mp1-m4-tee";reg = <0x10000000 0x40000>,<0x30000000 0x40000>,<0x38000000 0x10000>;st,proc-id = <0>;st,syscfg-rsc-tbl = <&tamp 0x144 0xFFFFFFFF>;st,syscfg-m4-state = <&tamp 0x148 0xFFFFFFFF>;- };
... -- 2.25.1