[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <PAXPR04MB8638D1504946559095E1CAB69530A@PAXPR04MB8638.eurprd04.prod.outlook.com>
Date: Mon, 10 Jul 2023 17:52:56 +0000
From: Pankaj Gupta <pankaj.gupta@....com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
"robh+dt@...nel.org" <robh+dt@...nel.org>,
"krzysztof.kozlowski+dt@...aro.org"
<krzysztof.kozlowski+dt@...aro.org>,
"conor+dt@...nel.org" <conor+dt@...nel.org>,
"shawnguo@...nel.org" <shawnguo@...nel.org>,
"s.hauer@...gutronix.de" <s.hauer@...gutronix.de>,
"kernel@...gutronix.de" <kernel@...gutronix.de>,
"festevam@...il.com" <festevam@...il.com>,
dl-linux-imx <linux-imx@....com>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
Gaurav Jain <gaurav.jain@....com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Daniel Baluta <daniel.baluta@....com>
Subject: RE: [EXT] Re: [PATCH v3 1/7] dt-bindings: arm: fsl: add mu binding
doc
> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
> Sent: Friday, June 16, 2023 6:51 PM
> To: Pankaj Gupta <pankaj.gupta@....com>; robh+dt@...nel.org;
> krzysztof.kozlowski+dt@...aro.org; conor+dt@...nel.org;
> shawnguo@...nel.org; s.hauer@...gutronix.de; kernel@...gutronix.de;
> festevam@...il.com; dl-linux-imx <linux-imx@....com>;
> devicetree@...r.kernel.org; linux-arm-kernel@...ts.infradead.org; Gaurav
> Jain <gaurav.jain@....com>; linux-kernel@...r.kernel.org; Daniel Baluta
> <daniel.baluta@....com>
> Subject: [EXT] Re: [PATCH v3 1/7] dt-bindings: arm: fsl: add mu 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 16/06/2023 20:11, Pankaj Gupta wrote:
> > The NXP i.MX Message Unit enables two processing elements to
> > communicate & co-ordinate with each other. This driver is used to
> > communicate between Application Core and NXP HSM IPs like NXP
> EdgeLock
> > Enclave etc.
> > It exists on some i.MX processors. e.g. i.MX8ULP, i.MX93 etc.
> >
> > Signed-off-by: Pankaj Gupta <pankaj.gupta@....com>
>
> I don't see reply to Daniel's concerns.
>
> I don't see improvements here based on the previous review you received.
> It seems you just ignored everything, right?
Replied to Daniel's concern.
>
> Limited review follows up because binding is not in the shape for upstream.
> Do some internal reviews prior sending it.
Done the internal review.
>
> > ---
> > .../bindings/arm/freescale/fsl,ele_mu.yaml | 144 ++++++++++++++++++
> > 1 file changed, 144 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/arm/freescale/fsl,ele_mu.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/arm/freescale/fsl,ele_mu.yaml
> > b/Documentation/devicetree/bindings/arm/freescale/fsl,ele_mu.yaml
> > new file mode 100644
> > index 000000000000..29e309a88899
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/arm/freescale/fsl,ele_mu.yaml
>
> No underscores, filename based on compatibles.
Accepted. Will correct in V4.
>
> > @@ -0,0 +1,144 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id:
> >
> +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> >
> +cetree.org%2Fschemas%2Farm%2Ffreescale%2Ffsl%2Cele_mu.yaml%23&d
> ata=05
> >
> +%7C01%7Cpankaj.gupta%40nxp.com%7C72b9e18a32a342bcf68c08db6e6c
> 8d5a%7C6
> >
> +86ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638225184750511073
> %7CUnknow
> >
> +n%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1h
> aWwiL
> >
> +CJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=5quVB0LAMCdjYghThl6PSI3
> CJPfuGtVoW
> > +AtN1gr4xm0%3D&reserved=0
> > +$schema:
> >
> +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> > +cetree.org%2Fmeta-
> schemas%2Fcore.yaml%23&data=05%7C01%7Cpankaj.gupta%
> >
> +40nxp.com%7C72b9e18a32a342bcf68c08db6e6c8d5a%7C686ea1d3bc2b4c
> 6fa92cd9
> >
> +9c5c301635%7C0%7C0%7C638225184750511073%7CUnknown%7CTWFpb
> GZsb3d8eyJWI
> >
> +joiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7
> C3000%
> >
> +7C%7C%7C&sdata=GyUGJThMnxLNEQX1guW9Q%2BdoVjMrvn%2FSupgJ7tu
> fkXk%3D&res
> > +erved=0
> > +
> > +title: NXP i.MX EdgeLock Enclave MUAP driver
>
> Drop driver.
Accepted. Will correct in V4.
>
> > +
> > +maintainers:
> > + - Pankaj Gupta <pankaj.gupta@....com>
> > +
> > +description: |
> > +
> > + NXP i.MX EdgeLock Enclave Message Unit Driver.
> > + The Messaging Unit module enables two processing elements within
> > + the SoC to communicate and coordinate by passing messages (e.g.,
> > + data, status and control) through its interfaces.
> > +
> > + The NXP i.MX EdgeLock Enclave Message Unit (ELE-MUAP) is
> > + specifically targeted for use between application core and
> > + Edgelocke Enclave. It allows to send messages to the EL Enclave using a
> shared mailbox.
> > +
> > + The messages must follow the protocol defined.
> > +
> > + Non-Secure + Secure
> > + |
> > + |
> > + +---------+ +-------------+ |
> > + | ele_mu.c+<---->+imx-mailbox.c| |
> > + | | | mailbox.c +<-->+------+ +------+
> > + +---+-----+ +-------------+ | MU X +<-->+ ELE |
> > + | +------+ +------+
> > + +----------------+ |
> > + | | |
> > + v v |
> > + logical logical |
> > + receiver waiter |
> > + + + |
> > + | | |
> > + | | |
> > + | +----+------+ |
> > + | | | |
> > + | | | |
> > + device_ctx device_ctx device_ctx |
> > + |
> > + User 0 User 1 User Y |
> > + +------+ +------+ +------+ |
> > + |misc.c| |misc.c| |misc.c| |
> > + kernel space +------+ +------+ +------+ |
> > + |
> > + +------------------------------------------------------ |
> > + | | | |
> > + userspace /dev/ele_muXch0 | | |
> > + /dev/ele_muXch1 | |
> > + /dev/ele_muXchY |
> > + |
> > +
> > + When a user sends a command to the ELE, it registers its device_ctx
> > + as waiter of a response from ELE.
> > +
> > + A user can be registered as receiver of command from the ELE.
> > + Create char devices in /dev as channels of the form /dev/ele_muXchY
> > + with X the id of the driver and Y for each users. It allows to send
> > + and receive messages to the NXP EdgeLock Enclave IP on NXP SoC,
> > + where current possible value, i.e., supported SoC(s) are imx8ulp, imx93.
> > +
> > +properties:
> > + compatible:
> > + enum:
> > + - fsl,imx-ele
> > + - fsl,imx93-ele
> > +
> > + mboxes:
> > + description:
> > + A list of phandles of TX MU channels followed by a list of phandles of
> > + RX MU channels. The number of expected tx and rx channels is 1 TX,
> and
> > + 1 RX channels. All MU channels must be within the same MU instance.
> > + Cross instances are not allowed. The MU instance to be used is
> S4MUAP
> > + for imx8ulp & imx93. Users need to ensure that used MU instance does
> not
> > + conflict with other execution environments.
> > + items:
> > + - description: TX0 MU channel
> > + - description: RX0 MU channel
> > +
> > + mbox-names:
> > + items:
> > + - const: tx
> > + - const: rx
> > +
> > + fsl,ele_mu_did:
>
> No underscores. Drop all properties not related to hardware.
Accepted. Will correct in V4.
>
>
> > + description:
> > + Owner of message-unit, is identified via Domain ID or did.
> > + allOf:
> > + - $ref: /schemas/types.yaml#/definitions/uint32
> > + - enum: [0, 1, 2, 3, 4, 5, 6, 7]
>
> That's not the syntax you can find. Open example-schema and rewrite your
> bindings.
Accepted. Will correct in V4.
Change it to:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ Owner of message-unit, is identified via Domain ID or did.
>
>
> > +
> > +examples:
> > + - |
> > + ele_mu: ele_mu {
>
> Node names should be generic. See also explanation and list of examples in
> DT specification:
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdevic
> etree-specification.readthedocs.io%2Fen%2Flatest%2Fchapter2-devicetree-
> basics.html%23generic-names-
> recommendation&data=05%7C01%7Cpankaj.gupta%40nxp.com%7C72b9e1
> 8a32a342bcf68c08db6e6c8d5a%7C686ea1d3bc2b4c6fa92cd99c5c301635%7
> C0%7C0%7C638225184750511073%7CUnknown%7CTWFpbGZsb3d8eyJWIjoi
> MC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C30
> 00%7C%7C%7C&sdata=wvrm2b84xq6KYEIEEGm9%2BCAHaDsXOfKlLvamsjh3
> jTg%3D&reserved=0
>
Accepted. Will correct in V4.
Will use the generic name "se-fw" for "secure-enclave firmware", instead of "ele-mu"
> > + compatible = "fsl,imx93-ele";
> > + mbox-names = "tx", "rx";
> > + mboxes = <&s4muap 2 0
> > + &s4muap 3 0>;
> > + fsl,ele_mu_id = <1>;
Used generic name and changed from "ele_mu_id" to "mu-id"
> > + fsl,ele_max_users = <4>;
> > + fsl,cmd_tag = /bits/ 8 <0x17>;
> > + fsl,rsp_tag = /bits/ 8 <0xe1>;
Removed the above 3 non-hw defined bindings.
> > + };
>
> Best regards,
> Krzysztof
Powered by blists - more mailing lists