> -----Original Message----- > From: Rob Herring <robh@xxxxxxxxxx> > Sent: Saturday, May 11, 2024 1:39 AM > To: Pankaj Gupta <pankaj.gupta@xxxxxxx> > Cc: Jonathan Corbet <corbet@xxxxxxx>; Krzysztof Kozlowski > <krzysztof.kozlowski+dt@xxxxxxxxxx>; Conor Dooley <conor+dt@xxxxxxxxxx>; > Shawn Guo <shawnguo@xxxxxxxxxx>; Sascha Hauer > <s.hauer@xxxxxxxxxxxxxx>; Pengutronix Kernel Team > <kernel@xxxxxxxxxxxxxx>; Fabio Estevam <festevam@xxxxxxxxx>; linux- > doc@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > devicetree@xxxxxxxxxxxxxxx; imx@xxxxxxxxxxxxxxx; linux-arm- > kernel@xxxxxxxxxxxxxxxxxxx > Subject: [EXT] Re: [PATCH 2/4] dt-bindings: arm: fsl: add imx-se-fw binding > doc > > Caution: This is an external email. Please take care when clicking links or > opening attachments. When in doubt, report the message using the 'Report > this email' button > > > On Fri, May 10, 2024 at 06:57:28PM +0530, Pankaj Gupta wrote: > > 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@xxxxxxx> > > --- > > .../devicetree/bindings/firmware/fsl,imx-se.yaml | 186 > +++++++++++++++++++++ > > 1 file changed, 186 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..a858ef6965cb > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/firmware/fsl,imx-se.yaml > > @@ -0,0 +1,186 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2 > > +--- > > +$id: > > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi > > +cetree.org%2Fschemas%2Ffirmware%2Ffsl%2Cimx- > se.yaml%23&data=05%7C02%7 > > > +Cpankaj.gupta%40nxp.com%7C2c17789530e9431069b308dc712d0e47%7C > 686ea1d3 > > > +bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638509685520936926%7CUnkn > own%7CTWF > > > +pbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJX > VCI6 > > > +Mn0%3D%7C0%7C%7C%7C&sdata=Rwof3k3K1OWimcI5ApRMiyvD%2FPXgk > b%2BRWrvx76J > > +YBtw%3D&reserved=0 > > +$schema: > > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi > > +cetree.org%2Fmeta- > schemas%2Fcore.yaml%23&data=05%7C02%7Cpankaj.gupta% > > > +40nxp.com%7C2c17789530e9431069b308dc712d0e47%7C686ea1d3bc2b4c > 6fa92cd9 > > > +9c5c301635%7C0%7C0%7C638509685520944809%7CUnknown%7CTWFpb > GZsb3d8eyJWI > > > +joiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7 > C0%7C% > > > +7C%7C&sdata=aile3WnYE69fuJHvAOXQARlqLV9roZDuDK63fCj2ZTE%3D&res > erved=0 > > + > > +title: NXP i.MX HW Secure Enclave(s) EdgeLock Enclave > > + > > +maintainers: > > + - Pankaj Gupta <pankaj.gupta@xxxxxxx> > > + > > +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: > > + '#address-cells': > > + const: 1 > > + > > + '#size-cells': > > + const: 0 > > + > > + compatible: > > + enum: > > + - fsl,imx8ulp-ele > > + - fsl,imx93-ele > > You basically have 0 properties in the parent node. What's the point of it? > Either just get rid of it and define the child nodes independently or make the > parent contain all the resources. > > > + > > +patternProperties: > > + "^[0-9a-z]*-if@[0-9]+$": > > unit-addresses are hex. Accepted. > > > + type: object > > + description: > > + Communication interface to secure-enclave node, that defines > hardware > > + properties to required to establish the communication. There can be > > + multiple interfaces to the same secure-enclave. Each interface is > > + enumerated with reg property. It optionally defines properties > > + depending on the compatible string and interface enum identifier. > > + > > + properties: > > + reg: > > + maxItems: 1 > > + description: Identifier of the communication interface to secure- > enclave. > > What are the identifiers based on? Identifier is used to identify a particular secure enclave interface, within the multiple secure enclave interfaces under same compatible string. Secure enclave interface is the combination of: - messaging unit (hardware), and - memory constraint applicable to a particular interface. > Is the value significant to s/w? Kind of looks like you just made up indices. For the below example: v2x { compatible = "fsl,imx95-v2x"; #address-cells = <1>; #size-cells = <0>; v2x-if@0 { // if it is not @0, then what identifier to be used to identify a particular node. reg = <0x0>; mboxes = <&v2x_mu 0 0>, <&v2x_mu 1 0>; mbox-names = "tx", "rx"; }; v2x-if@1 { reg = <0x1>; mboxes = <&v2x_mu6 0 0>, <&v2x_mu6 1 0>; mbox-names = "txdb", "rxdb"; }; v2x-if@2 { reg = <0x2>; mboxes = <&v2x_mu7 0 0>, <&v2x_mu7 1 0>; mbox-names = "tx", "rx"; }; }; s/w needs to differentiate one node from another node, for the same compatible string for the SoC. > > How many child nodes do you have? Is it fixed per SoC? It is fixed for a particular SoC. While number of child nodes varies from one SoC to another SoC. > > > + > > + mboxes: > > + description: contain a list of phandles to mailboxes. > > + items: > > + - description: Specify the mailbox used to send message to se > firmware > > + - 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. > > + (see bindings/reserved-memory/reserved-memory.txt) > > + > > + sram: > > + description: contains a list of phandles to sram. > > + $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. > > + > > + required: > > + - reg > > + - mboxes > > + - mbox-names > > + > > +allOf: > > + # memory-region > > + - if: > > + properties: > > + compatible: > > + contains: > > + enum: > > + - fsl,imx8ulp-ele > > + - fsl,imx93-ele > > What else would they contain? Those are the only compatibles defined here. It will have multiple child nodes, but fixed number of child node for a particular SoC. > > > + then: > > + patternProperties: > > + "^[0-9a-z]*-if@[0-9]+$": > > + allOf: > > + - if: > > These conditionals are hard to follow. Probably a sign some of this needs to be > separate or simplified. I am ready to separate and simplify. Please help by sharing an example on how to go about this change. Probably an example that helps me understand your point and then will start to work on this comment. It is highly appreciated. Thanks. > > > + properties: > > + reg: > > + items: > > + - enum: > > + - 0 > > + then: > > + required: > > + - memory-region > > + else: > > + not: > > + required: > > + - memory-region > > + # sram > > + - if: > > + properties: > > + compatible: > > + contains: > > + enum: > > + - fsl,imx8ulp-ele > > + then: > > + patternProperties: > > + "^[0-9a-z]*-if@[0-9]+$": > > + allOf: > > + - if: > > + properties: > > + reg: > > + items: > > + - enum: > > + - 0 > > + then: > > + required: > > + - sram > > + else: > > + not: > > + required: > > + - sram > > + > > +additionalProperties: false
