diff mbox series

[v4,2/5] dt-bindings: arm: fsl: add imx-se-fw binding doc

Message ID 20240705-imx-se-if-v4-2-52d000e18a1d@nxp.com (mailing list archive)
State Superseded
Headers show
Series Changes in v4: | expand

Commit Message

Pankaj Gupta July 5, 2024, 1:52 p.m. UTC
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   | 133 +++++++++++++++++++++
 1 file changed, 133 insertions(+)

Comments

Krzysztof Kozlowski July 7, 2024, 12:34 p.m. UTC | #1
On 05/07/2024 15:52, 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.

This binding is not improving, even though it is v5.

> 
> Signed-off-by: Pankaj Gupta <pankaj.gupta@nxp.com>
> ---
>  .../devicetree/bindings/firmware/fsl,imx-se.yaml   | 133 +++++++++++++++++++++
>  1 file changed, 133 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..b9018645101d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/firmware/fsl,imx-se.yaml
> @@ -0,0 +1,133 @@
> +# 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: "firmware@[0-9a-f]+$"
> +
> +  compatible:
> +    enum:
> +      - fsl,imx8ulp-se
> +      - fsl,imx93-se
> +      - fsl,imx95-se
> +
> +  reg:
> +    maxItems: 1
> +    description: Identifier of the communication interface to secure-enclave.
> +
> +  mboxes:
> +    description: contain a list of phandles to mailboxes.

Drop, obvious.

> +    items:
> +      - description: Specify the mailbox used to send message to se firmware
> +      - description: Specify the mailbox used to receive message from se firmware

Drop redundant/obvious parts.

So two mailboxes?

> +
> +  mbox-names:
> +    items:
> +      - const: tx
> +      - const: rx
> +      - const: txdb
> +      - const: rxdb

4 mailboxes? This cannot be different.

> +    minItems: 2
> +
> +  memory-region:
> +    description: contains the phandle to reserved external memory.

Drop

> +    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 the phandle to sram.

Drop

> +    items:
> +      - description: Phandle to the device SRAM. It is an optional property
> +          based on compatible and identifier to communication interface.
> +
> +required:
> +  - compatible
> +  - reg
> +  - mboxes
> +  - mbox-names
> +
> +additionalProperties: false

Keep it after allOf block


> +
> +allOf:
> +  # memory-region
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - fsl,imx8ulp-se
> +              - fsl,imx93-se
> +    then:
> +      required:
> +        - memory-region
> +    else:
> +      properties:
> +        memory-region: false
> +
> +  # sram
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - fsl,imx8ulp-se
> +    then:
> +      required:
> +        - sram
> +
> +    else:
> +      properties:
> +        sram: false
> +
> +examples:
> +  - |
> +    firmware {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +      firmware@0 {
> +        compatible = "fsl,imx95-se";
> +        reg = <0x0>;
> +        mboxes = <&ele_mu0 0 0>, <&ele_mu0 1 0>;
> +        mbox-names = "tx", "rx";
> +      };

One example is enough.


Best regards,
Krzysztof
Pankaj Gupta July 8, 2024, 5:50 a.m. UTC | #2
> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Sunday, July 7, 2024 6:05 PM
> To: Pankaj Gupta <pankaj.gupta@nxp.com>; Jonathan Corbet
> <corbet@lwn.net>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski
> <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Shawn Guo
> <shawnguo@kernel.org>; Sascha Hauer <s.hauer@pengutronix.de>;
> Pengutronix Kernel Team <kernel@pengutronix.de>; Fabio Estevam
> <festevam@gmail.com>; Rob Herring <robh+dt@kernel.org>; Krzysztof
> Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> Cc: 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 v4 2/5] 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 05/07/2024 15:52, 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.
> 
> This binding is not improving, even though it is v5.

I have disposed-off all the comments received by Conor and Rob.
Expecting there Reviewed-By on the v4, if they had no more comments.

I will ensure to address your comments on this mail chain, in the best way I could.

> 
> >
> > Signed-off-by: Pankaj Gupta <pankaj.gupta@nxp.com>
> > ---
> >  .../devicetree/bindings/firmware/fsl,imx-se.yaml   | 133
> +++++++++++++++++++++
> >  1 file changed, 133 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..b9018645101d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/firmware/fsl,imx-se.yaml
> > @@ -0,0 +1,133 @@
> > +# 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%7Cb43d043a91934c0c094c08dc9e813c2f%7C
> 686ea1d3
> >
> +bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638559525096552364%7CUn
> known%7CTWF
> >
> +pbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJ
> XVCI6
> >
> +Mn0%3D%7C0%7C%7C%7C&sdata=zmZkwos2LRrAdtPBr0pCJxCrztncKCQS2
> 4ECYMIjSnw
> > +%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%7Cb43d043a91934c0c094c08dc9e813c2f%7C686ea1d3bc2b4
> c6fa92cd9
> >
> +9c5c301635%7C0%7C0%7C638559525096560485%7CUnknown%7CTWF
> pbGZsb3d8eyJWI
> >
> +joiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7
> C0%7C%
> >
> +7C%7C&sdata=WNl9M3G%2BPIl4xPx4w74Tsv7OIBkTRiR0Er5uj7PAt5o%3D
> &reserved
> > +=0
> > +
> > +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: "firmware@[0-9a-f]+$"
> > +
> > +  compatible:
> > +    enum:
> > +      - fsl,imx8ulp-se
> > +      - fsl,imx93-se
> > +      - fsl,imx95-se
> > +
> > +  reg:
> > +    maxItems: 1
> > +    description: Identifier of the communication interface to secure-enclave.
> > +
> > +  mboxes:
> > +    description: contain a list of phandles to mailboxes.
> 
> Drop, obvious.
Will remove the line "- 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
> 
> Drop redundant/obvious parts.
Will replace above two description line with the below two.
      - description: mailbox phandle to send message to se firmware
      - description: mailbox phandle to receive message from se firmware

> 
> So two mailboxes?
Two handles of the same mailbox.

> 
> > +
> > +  mbox-names:
> > +    items:
> > +      - const: tx
> > +      - const: rx
> > +      - const: txdb
> > +      - const: rxdb
> 
> 4 mailboxes? This cannot be different.
mbox-names can have any of the above mentioned 4 values.
And  two values are minimum, required.

> 
> > +    minItems: 2
> > +
> > +  memory-region:
> > +    description: contains the phandle to reserved external memory.
> 
> Drop
Will remove the line " description: contains the phandle 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 the phandle to sram.
> 
> Drop
Will remove " description: contains the phandle to sram."

> 
> > +    items:
> > +      - description: Phandle to the device SRAM. It is an optional property
> > +          based on compatible and identifier to communication interface.
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - mboxes
> > +  - mbox-names
> > +
> > +additionalProperties: false
> 
> Keep it after allOf block

In V2, it is after allOf block.
In previous comments, it was commented to place allOf block, after required.
I am little confused.

> 
> 
> > +
> > +allOf:
> > +  # memory-region
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - fsl,imx8ulp-se
> > +              - fsl,imx93-se
> > +    then:
> > +      required:
> > +        - memory-region
> > +    else:
> > +      properties:
> > +        memory-region: false
> > +
> > +  # sram
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - fsl,imx8ulp-se
> > +    then:
> > +      required:
> > +        - sram
> > +
> > +    else:
> > +      properties:
> > +        sram: false
> > +
> > +examples:
> > +  - |
> > +    firmware {
> > +      #address-cells = <1>;
> > +      #size-cells = <0>;
> > +      firmware@0 {
> > +        compatible = "fsl,imx95-se";
> > +        reg = <0x0>;
> > +        mboxes = <&ele_mu0 0 0>, <&ele_mu0 1 0>;
> > +        mbox-names = "tx", "rx";
> > +      };
> 
> One example is enough.
Accepted.

> 
> 
> Best regards,
> Krzysztof
Krzysztof Kozlowski July 8, 2024, 6:16 a.m. UTC | #3
On 08/07/2024 07:50, Pankaj Gupta wrote:
>> Drop redundant/obvious parts.
> Will replace above two description line with the below two.
>       - description: mailbox phandle to send message to se firmware
>       - description: mailbox phandle to receive message from se firmware
> 
>>
>> So two mailboxes?
> Two handles of the same mailbox.
> 
>>
>>> +
>>> +  mbox-names:
>>> +    items:
>>> +      - const: tx
>>> +      - const: rx
>>> +      - const: txdb
>>> +      - const: rxdb
>>
>> 4 mailboxes? This cannot be different.
> mbox-names can have any of the above mentioned 4 values.
> And  two values are minimum, required.

No, I said it cannot be different. If two are minimum, then you can have
here 4, right? But earlier you said you can have only 2. It does not
make any sense.

> 
>>
>>> +    minItems: 2
>>> +
>>> +  memory-region:
>>> +    description: contains the phandle to reserved external memory.
>>
>> Drop
> Will remove the line " description: contains the phandle 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 the phandle to sram.
>>
>> Drop
> Will remove " description: contains the phandle to sram."
> 
>>
>>> +    items:
>>> +      - description: Phandle to the device SRAM. It is an optional property
>>> +          based on compatible and identifier to communication interface.
>>> +
>>> +required:
>>> +  - compatible
>>> +  - reg
>>> +  - mboxes
>>> +  - mbox-names
>>> +
>>> +additionalProperties: false
>>
>> Keep it after allOf block
> 
> In V2, it is after allOf block.
> In previous comments, it was commented to place allOf block, after required.
> I am little confused.

So why did you implement it entirely different? Read the comment from
Conor and from me again. I am sorry, but repeating the same three times
(once by Conor, twice by me) is quite a waste of time.

Open example-schema. How is it done there?

Best regards,
Krzysztof
Pankaj Gupta July 8, 2024, 11:19 a.m. UTC | #4
> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Monday, July 8, 2024 11:46 AM
> To: Pankaj Gupta <pankaj.gupta@nxp.com>; Jonathan Corbet
> <corbet@lwn.net>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski
> <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Shawn Guo
> <shawnguo@kernel.org>; Sascha Hauer <s.hauer@pengutronix.de>;
> Pengutronix Kernel Team <kernel@pengutronix.de>; Fabio Estevam
> <festevam@gmail.com>; Rob Herring <robh+dt@kernel.org>; Krzysztof
> Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> Cc: 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 v4 2/5] 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 08/07/2024 07:50, Pankaj Gupta wrote:
> >> Drop redundant/obvious parts.
> > Will replace above two description line with the below two.
> >       - description: mailbox phandle to send message to se firmware
> >       - description: mailbox phandle to receive message from se
> > firmware
> >
> >>
> >> So two mailboxes?
> > Two handles of the same mailbox.
> >
> >>
> >>> +
> >>> +  mbox-names:
> >>> +    items:
> >>> +      - const: tx
> >>> +      - const: rx
> >>> +      - const: txdb
> >>> +      - const: rxdb
> >>
> >> 4 mailboxes? This cannot be different.
> > mbox-names can have any of the above mentioned 4 values.
> > And  two values are minimum, required.
> 
> No, I said it cannot be different. If two are minimum, then you can have here
> 4, right? But earlier you said you can have only 2. It does not make any sense.

Understood.  Will fix this with following changes.

-      - const: tx
-      - const: rx
-      - const: txdb
-      - const: rxdb
-    minItems: 2
+      - enum:
+          - tx
+          - txdb
+      - enum:
+          - rx
+          - rxdb


> 
> >
> >>
> >>> +    minItems: 2
> >>> +
> >>> +  memory-region:
> >>> +    description: contains the phandle to reserved external memory.
> >>
> >> Drop
> > Will remove the line " description: contains the phandle 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 the phandle to sram.
> >>
> >> Drop
> > Will remove " description: contains the phandle to sram."
> >
> >>
> >>> +    items:
> >>> +      - description: Phandle to the device SRAM. It is an optional property
> >>> +          based on compatible and identifier to communication interface.
> >>> +
> >>> +required:
> >>> +  - compatible
> >>> +  - reg
> >>> +  - mboxes
> >>> +  - mbox-names
> >>> +
> >>> +additionalProperties: false
> >>
> >> Keep it after allOf block
> >
> > In V2, it is after allOf block.
> > In previous comments, it was commented to place allOf block, after
> required.
> > I am little confused.
> 
> So why did you implement it entirely different? Read the comment from Conor
> and from me again. I am sorry, but repeating the same three times (once by
> Conor, twice by me) is quite a waste of time.
> 
> Open example-schema. How is it done there?

Understood the point. Referred to the example-schema.
Will correct as per the example-schema.

> 
> Best regards,
> Krzysztof
Krzysztof Kozlowski July 8, 2024, 11:23 a.m. UTC | #5
On 08/07/2024 13:19, Pankaj Gupta wrote:
>>>> 4 mailboxes? This cannot be different.
>>> mbox-names can have any of the above mentioned 4 values.
>>> And  two values are minimum, required.
>>
>> No, I said it cannot be different. If two are minimum, then you can have here
>> 4, right? But earlier you said you can have only 2. It does not make any sense.
> 
> Understood.  Will fix this with following changes.
> 
> -      - const: tx
> -      - const: rx
> -      - const: txdb
> -      - const: rxdb
> -    minItems: 2
> +      - enum:
> +          - tx
> +          - txdb
> +      - enum:
> +          - rx
> +          - rxdb

OK, this is now clear and answer is also clear:
No, you cannot have different names, except some rare cases. Please
explain why this should be an exception?

Best regards,
Krzysztof
Pankaj Gupta July 8, 2024, 11:54 a.m. UTC | #6
> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Monday, July 8, 2024 4:54 PM
> To: Pankaj Gupta <pankaj.gupta@nxp.com>; Jonathan Corbet
> <corbet@lwn.net>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski
> <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Shawn Guo
> <shawnguo@kernel.org>; Sascha Hauer <s.hauer@pengutronix.de>;
> Pengutronix Kernel Team <kernel@pengutronix.de>; Fabio Estevam
> <festevam@gmail.com>; Rob Herring <robh+dt@kernel.org>; Krzysztof
> Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> Cc: 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 v4 2/5] 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 08/07/2024 13:19, Pankaj Gupta wrote:
> >>>> 4 mailboxes? This cannot be different.
> >>> mbox-names can have any of the above mentioned 4 values.
> >>> And  two values are minimum, required.
> >>
> >> No, I said it cannot be different. If two are minimum, then you can
> >> have here 4, right? But earlier you said you can have only 2. It does not
> make any sense.
> >
> > Understood.  Will fix this with following changes.
> >
> > -      - const: tx
> > -      - const: rx
> > -      - const: txdb
> > -      - const: rxdb
> > -    minItems: 2
> > +      - enum:
> > +          - tx
> > +          - txdb
> > +      - enum:
> > +          - rx
> > +          - rxdb
> 
> OK, this is now clear and answer is also clear:
> No, you cannot have different names, except some rare cases.
> Please explain why this should be an exception?

The name suggests the type of protocol for:
- sending:    tx or txdb (with doorbell).
- receiving:  rx & rxdb  (with doorbell).

NXP IP SECO on i.MX8DXL/8QXP supports txdb & rxdb. Hence added these names.

It is just for readability to understand from platform's DTSI file.


> 
> Best regards,
> Krzysztof
Krzysztof Kozlowski July 8, 2024, 12:56 p.m. UTC | #7
On 08/07/2024 13:54, Pankaj Gupta wrote:
> 
> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzk@kernel.org>
>> Sent: Monday, July 8, 2024 4:54 PM
>> To: Pankaj Gupta <pankaj.gupta@nxp.com>; Jonathan Corbet
>> <corbet@lwn.net>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski
>> <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Shawn Guo
>> <shawnguo@kernel.org>; Sascha Hauer <s.hauer@pengutronix.de>;
>> Pengutronix Kernel Team <kernel@pengutronix.de>; Fabio Estevam
>> <festevam@gmail.com>; Rob Herring <robh+dt@kernel.org>; Krzysztof
>> Kozlowski <krzysztof.kozlowski+dt@linaro.org>
>> Cc: 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 v4 2/5] 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 08/07/2024 13:19, Pankaj Gupta wrote:
>>>>>> 4 mailboxes? This cannot be different.
>>>>> mbox-names can have any of the above mentioned 4 values.
>>>>> And  two values are minimum, required.
>>>>
>>>> No, I said it cannot be different. If two are minimum, then you can
>>>> have here 4, right? But earlier you said you can have only 2. It does not
>> make any sense.
>>>
>>> Understood.  Will fix this with following changes.
>>>
>>> -      - const: tx
>>> -      - const: rx
>>> -      - const: txdb
>>> -      - const: rxdb
>>> -    minItems: 2
>>> +      - enum:
>>> +          - tx
>>> +          - txdb
>>> +      - enum:
>>> +          - rx
>>> +          - rxdb
>>
>> OK, this is now clear and answer is also clear:
>> No, you cannot have different names, except some rare cases.
>> Please explain why this should be an exception?
> 
> The name suggests the type of protocol for:
> - sending:    tx or txdb (with doorbell).
> - receiving:  rx & rxdb  (with doorbell).
> 
> NXP IP SECO on i.MX8DXL/8QXP supports txdb & rxdb. Hence added these names.
> 
> It is just for readability to understand from platform's DTSI file.

Then no, that's not a reason to break fixed naming rule. These are
supposed to be just tx and rx.

Best regards,
Krzysztof
diff mbox series

Patch

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..b9018645101d
--- /dev/null
+++ b/Documentation/devicetree/bindings/firmware/fsl,imx-se.yaml
@@ -0,0 +1,133 @@ 
+# 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: "firmware@[0-9a-f]+$"
+
+  compatible:
+    enum:
+      - fsl,imx8ulp-se
+      - fsl,imx93-se
+      - fsl,imx95-se
+
+  reg:
+    maxItems: 1
+    description: Identifier of the communication interface to secure-enclave.
+
+  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 the phandle 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 the phandle to sram.
+    items:
+      - description: Phandle to the device SRAM. It is an optional property
+          based on compatible and identifier to communication interface.
+
+required:
+  - compatible
+  - reg
+  - mboxes
+  - mbox-names
+
+additionalProperties: false
+
+allOf:
+  # memory-region
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - fsl,imx8ulp-se
+              - fsl,imx93-se
+    then:
+      required:
+        - memory-region
+    else:
+      properties:
+        memory-region: false
+
+  # sram
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - fsl,imx8ulp-se
+    then:
+      required:
+        - sram
+
+    else:
+      properties:
+        sram: false
+
+examples:
+  - |
+    firmware {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      firmware@0 {
+        compatible = "fsl,imx95-se";
+        reg = <0x0>;
+        mboxes = <&ele_mu0 0 0>, <&ele_mu0 1 0>;
+        mbox-names = "tx", "rx";
+      };
+      firmware@3 {
+        compatible = "fsl,imx95-se";
+        reg = <0x3>;
+        mboxes = <&v2x_mu 0 0>, <&v2x_mu 1 0>;
+        mbox-names = "tx", "rx";
+      };
+      firmware@4 {
+        compatible = "fsl,imx95-se";
+        reg = <0x4>;
+        mboxes = <&v2x_mu6 0 0>, <&v2x_mu6 1 0>;
+        mbox-names = "tx", "rx";
+      };
+      firmware@5 {
+        compatible = "fsl,imx95-se";
+        reg = <0x5>;
+        mboxes = <&v2x_mu7 0 0>, <&v2x_mu7 1 0>;
+        mbox-names = "tx", "rx";
+      };
+    };
+...