Hi Conor,

quoted

quoted

+properties:
+  $nodename:
+    pattern: "^[0-9a-z]*-if@[0-9a-f]+$"

Just "firmware@<hex>" please.

Modified as per your suggestion, 
-    pattern: "^[0-9a-z]*-if@[0-9a-f]+$"
+    pattern: "^[0-9a-z]*-if@<hex>"

encountering the following error:
/home/yuktilab/linux_bkp/Documentation/devicetree/bindings/firmware/fsl,imx-se.example.dtb: ele-if@0: $nodename:0: 'ele-if@0' does not match '^[0-9a-z]*-if@<hex>'
	from schema $id: http://devicetree.org/schemas/firmware/fsl,imx-se.yaml#
/home/yuktilab/linux_bkp/Documentation/devicetree/bindings/firmware/fsl,imx-se.example.dtb: v2x-if@3: $nodename:0: 'v2x-if@3' does not match '^[0-9a-z]*-if@<hex>'
	from schema $id: http://devicetree.org/schemas/firmware/fsl,imx-se.yaml#
/home/yuktilab/linux_bkp/Documentation/devicetree/bindings/firmware/fsl,imx-se.example.dtb: v2x-if@4: $nodename:0: 'v2x-if@4' does not match '^[0-9a-z]*-if@<hex>'
	from schema $id: http://devicetree.org/schemas/firmware/fsl,imx-se.yaml#
/home/yuktilab/linux_bkp/Documentation/devicetree/bindings/firmware/fsl,imx-se.example.dtb: v2x-if@5: $nodename:0: 'v2x-if@5' does not match '^[0-9a-z]*-if@<hex>'
	from schema $id: http://devicetree.org/schemas/firmware/fsl,imx-se.yaml#

Please help and guide to resolve this comment, correctly.
Highly appreciated. Thanks.

Regards
Pankaj

-----Original Message-----
From: Pankaj Gupta
Sent: Tuesday, June 18, 2024 4:29 PM
To: Conor Dooley <conor@kernel.org>
Cc: Jonathan Corbet <corbet@lwn.net>; Rob Herring <robh@kernel.org>;
Krzysztof Kozlowski [off-list ref]; Conor Dooley
[off-list ref]; Shawn Guo [off-list ref]; Sascha Hauer
[off-list ref]; Pengutronix Kernel Team
[off-list ref]; Fabio Estevam [off-list ref]; Rob
Herring [off-list ref]; Krzysztof Kozlowski
[off-list ref]; linux-doc@vger.kernel.org; linux-
kernel@vger.kernel.org; devicetree@vger.kernel.org; imx@lists.linux.dev;
linux-arm-kernel@lists.infradead.org
Subject: RE: [EXT] Re: [PATCH v3 2/5] dt-bindings: arm: fsl: add imx-se-fw
binding doc

quoted

-----Original Message-----
From: Conor Dooley <conor@kernel.org>
Sent: Monday, June 17, 2024 10:07 PM
To: Pankaj Gupta <pankaj.gupta@nxp.com>
Cc: Jonathan Corbet <corbet@lwn.net>; Rob Herring <robh@kernel.org>;
Krzysztof Kozlowski [off-list ref]; Conor Dooley
[off-list ref]; Shawn Guo [off-list ref]; Sascha Hauer
[off-list ref]; Pengutronix Kernel Team
[off-list ref]; Fabio Estevam [off-list ref]; Rob
Herring [off-list ref]; Krzysztof Kozlowski
[off-list ref]; linux-doc@vger.kernel.org; linux-
kernel@vger.kernel.org; devicetree@vger.kernel.org;
imx@lists.linux.dev; linux-arm-kernel@lists.infradead.org
Subject: [EXT] Re: [PATCH v3 2/5] dt-bindings: arm: fsl: add imx-se-fw
binding doc

On Mon, Jun 17, 2024 at 12:59:40PM +0530, Pankaj Gupta wrote:

quoted

The NXP security hardware IP(s) like: i.MX EdgeLock Enclave, V2X
etc., creates an embedded secure enclave within the SoC boundary to
enable features like:
- HSM
- SHE
- V2X

Secure-Enclave(s) communication interface are typically via message
unit, i.e., based on mailbox linux kernel driver. This driver
enables communication ensuring well defined message sequence
protocol between Application Core and enclave's firmware.

Driver configures multiple misc-device on the MU, for multiple
user-space applications, to be able to communicate over single MU.

It exists on some i.MX processors. e.g. i.MX8ULP, i.MX93 etc.

Signed-off-by: Pankaj Gupta <pankaj.gupta@nxp.com>
---
 .../devicetree/bindings/firmware/fsl,imx-se.yaml   | 160

+++++++++++++++++++++

quoted

 1 file changed, 160 insertions(+)

diff --git
a/Documentation/devicetree/bindings/firmware/fsl,imx-se.yaml
b/Documentation/devicetree/bindings/firmware/fsl,imx-se.yaml
new file mode 100644
index 000000000000..60ad1c4a3dfa

--- /dev/null
+++ b/Documentation/devicetree/bindings/firmware/fsl,imx-se.yaml

@@ -0,0 +1,160 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
+---
+$id: http://devicetree.org/schemas/firmware/fsl,imx-se.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP i.MX HW Secure Enclave(s) EdgeLock Enclave
+
+maintainers:
+  - Pankaj Gupta <pankaj.gupta@nxp.com>
+
+description: |
+  NXP's SoC may contain one or multiple embedded secure-enclave HW
+  IP(s) like i.MX EdgeLock Enclave, V2X etc. These NXP's HW IP(s)
+  enables features like
+    - Hardware Security Module (HSM),
+    - Security Hardware Extension (SHE), and
+    - Vehicular to Anything (V2X)
+
+  Communication interface to the secure-enclaves is based on the
+ messaging unit(s).
+
+properties:
+  $nodename:
+    pattern: "^[0-9a-z]*-if@[0-9a-f]+$"

Just "firmware@<hex>" please.

Modified as per your suggestion,
-    pattern: "^[0-9a-z]*-if@[0-9a-f]+$"
+    pattern: "^[0-9a-z]*-if@<hex>"

encountering the following error:
/home/yuktilab/linux_bkp/Documentation/devicetree/bindings/firmware/fsl,i
mx-se.example.dtb: ele-if@0: $nodename:0: 'ele-if@0' does not match '^[0-
9a-z]*-if@<hex>'
	from schema $id: http://devicetree.org/schemas/firmware/fsl,imx-
se.yaml#
/home/yuktilab/linux_bkp/Documentation/devicetree/bindings/firmware/fsl,i
mx-se.example.dtb: v2x-if@3: $nodename:0: 'v2x-if@3' does not match '^[0-
9a-z]*-if@<hex>'
	from schema $id: http://devicetree.org/schemas/firmware/fsl,imx-
se.yaml#
/home/yuktilab/linux_bkp/Documentation/devicetree/bindings/firmware/fsl,i
mx-se.example.dtb: v2x-if@4: $nodename:0: 'v2x-if@4' does not match '^[0-
9a-z]*-if@<hex>'
	from schema $id: http://devicetree.org/schemas/firmware/fsl,imx-
se.yaml#
/home/yuktilab/linux_bkp/Documentation/devicetree/bindings/firmware/fsl,i
mx-se.example.dtb: v2x-if@5: $nodename:0: 'v2x-if@5' does not match '^[0-
9a-z]*-if@<hex>'
	from schema $id: http://devicetree.org/schemas/firmware/fsl,imx-
se.yaml#

Please help and guide to resolve this comment, correctly.
Highly appreciated. Thanks.

quoted

quoted

+
+  compatible:
+    enum:
+      - fsl,imx8ulp-se
+      - fsl,imx93-se
+      - fsl,imx95-se
+
+  reg:
+    maxItems: 1
+    description: Identifier of the communication interface to secure-

enclave.

quoted

quoted

+
+  mboxes:
+    description: contain a list of phandles to mailboxes.
+    items:
+      - description: Specify the mailbox used to send message to se

firmware

quoted

quoted

+      - description: Specify the mailbox used to receive message
+ from se firmware
+
+  mbox-names:
+    items:
+      - const: tx
+      - const: rx
+      - const: txdb
+      - const: rxdb
+    minItems: 2
+
+  memory-region:
+    description: contains a list of phandles to reserved external memory.
+    items:
+      - description: It is used by secure-enclave firmware. It is an optional
+          property based on compatible and identifier to
+ communication

interface.

quoted

+          (see bindings/reserved-memory/reserved-memory.txt)
+
+  sram:
+    description: contains a list of phandles to sram.

There's only 1 phandle allowed, don't describe it as a list.
Same for memory-region.

Accepted. Will do the following changes.
   memory-region:
-    description: contains a list of phandles to reserved external memory.
+    description: contains the phandle to reserved external memory.

   sram:
-    description: contains a list of phandles to sram.
-    $ref: /schemas/types.yaml#/definitions/phandle-array
+    description: contains the phandle to sram.
     items:

quoted

quoted

+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    items:
+      - description: Phandle to the device SRAM. It is an optional property
+          based on compatible and identifier to communication interface.
+
+allOf:
+  # memory-region
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - fsl,imx8ulp-se
+              - fsl,imx93-se
+    then:
+      required:
+        - memory-region

quoted

+    else:
+      not:
+        required:
+          - memory-region

Use

else: properties: memory-region: false

Same for sram.

Accepted will be corrected in V4.
# memory-region
-      not:
-        required:
-          - memory-region
+      properties:
+        memory-region: false

   # sram
     else:
-      not:
-        required:
-          - sram
+      properties:
+        sram: false

quoted

Sort the allOf after required.

Accepted. Will move allOf after "additionalProperties:"

quoted

quoted

+
+  # sram
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - fsl,imx8ulp-se
+    then:
+      required:
+        - sram
+    else:
+      not:
+        required:
+          - sram
+
+required:
+  - compatible
+  - reg
+  - mboxes
+  - mbox-names
+
+additionalProperties: false
+
+examples:
+  - |
+    firmware {

You've made up these firmware "buses" here, what purpose do they
serve, other than allowing you to have a reg property?

True.
Additionally, these are firmware nodes, these nodes are semantically put in
firmware "buses".

quoted

quoted

+      #address-cells = <1>;
+      #size-cells = <0>;
+      ele-if@0 {
+        compatible = "fsl,imx8ulp-se";
+        reg = <0x0>;

What does the reg property even do? Is it ever more than 0?
Can this information be provided as a mbox cell?

Yes, there are more than 0 nodes for i.MX95 platforms, i.e., total of around 7
such nodes.
Reg property will help to identify the node id, by other kernel management
layer like NVMEM (patches to follow).

quoted

quoted

+        mboxes = <&s4muap 0 0>, <&s4muap 1 0>;
+        mbox-names = "tx", "rx";
+        sram = <&sram0>;
+        memory-region = <&ele_reserved>;
+      };
+    };
+  - |
+    firmware {

These examples are all basically the same, drop all but one.

Ok, will keep the example of i.MX95.

quoted

Thanks,
Conor.

quoted

+      #address-cells = <1>;
+      #size-cells = <0>;
+      ele-if@0 {
+        compatible = "fsl,imx93-se";
+        reg = <0x0>;
+        mboxes = <&s4muap 0 0>, <&s4muap 1 0>;
+        mbox-names = "tx", "rx";
+        memory-region = <&ele_reserved>;
+      };
+    };
+  - |
+    firmware {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      ele-if@0 {
+        compatible = "fsl,imx95-se";
+        reg = <0x0>;
+        mboxes = <&ele_mu0 0 0>, <&ele_mu0 1 0>;
+        mbox-names = "tx", "rx";
+      };
+      v2x-if@3 {
+        compatible = "fsl,imx95-se";
+        reg = <0x3>;
+        mboxes = <&v2x_mu 0 0>, <&v2x_mu 1 0>;
+        mbox-names = "tx", "rx";
+      };
+      v2x-if@4 {
+        compatible = "fsl,imx95-se";
+        reg = <0x4>;
+        mboxes = <&v2x_mu6 0 0>, <&v2x_mu6 1 0>;
+        mbox-names = "tx", "rx";
+      };
+      v2x-if@5 {
+        compatible = "fsl,imx95-se";
+        reg = <0x5>;
+        mboxes = <&v2x_mu7 0 0>, <&v2x_mu7 1 0>;
+        mbox-names = "tx", "rx";
+      };
+    };
+...

--
2.34.1