[V2,1/2] DT: mailbox: add binding doc for the ARM SMC mailbox
diff mbox series

Message ID 20190603083005.4304-2-peng.fan@nxp.com
State New
Headers show
Series
  • mailbox: arm: introduce smc triggered mailbox
Related show

Commit Message

Peng Fan June 3, 2019, 8:30 a.m. UTC
From: Peng Fan <peng.fan@nxp.com>

The ARM SMC mailbox binding describes a firmware interface to trigger
actions in software layers running in the EL2 or EL3 exception levels.
The term "ARM" here relates to the SMC instruction as part of the ARM
instruction set, not as a standard endorsed by ARM Ltd.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---

V2:
Introduce interrupts as a property.

V1:
arm,func-ids is still kept as an optional property, because there is no
defined SMC funciton id passed from SCMI. So in my test, I still use
arm,func-ids for ARM SIP service.

 .../devicetree/bindings/mailbox/arm-smc.txt        | 101 +++++++++++++++++++++
 1 file changed, 101 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/arm-smc.txt

Comments

Florian Fainelli June 3, 2019, 4:22 p.m. UTC | #1
On 6/3/19 1:30 AM, peng.fan@nxp.com wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> The ARM SMC mailbox binding describes a firmware interface to trigger
> actions in software layers running in the EL2 or EL3 exception levels.
> The term "ARM" here relates to the SMC instruction as part of the ARM
> instruction set, not as a standard endorsed by ARM Ltd.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> 
> V2:
> Introduce interrupts as a property.
> 
> V1:
> arm,func-ids is still kept as an optional property, because there is no
> defined SMC funciton id passed from SCMI. So in my test, I still use
> arm,func-ids for ARM SIP service.
> 
>  .../devicetree/bindings/mailbox/arm-smc.txt        | 101 +++++++++++++++++++++
>  1 file changed, 101 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mailbox/arm-smc.txt
> 
> diff --git a/Documentation/devicetree/bindings/mailbox/arm-smc.txt b/Documentation/devicetree/bindings/mailbox/arm-smc.txt
> new file mode 100644
> index 000000000000..401887118c09
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mailbox/arm-smc.txt
> @@ -0,0 +1,101 @@
> +ARM SMC Mailbox Interface
> +=========================
> +
> +This mailbox uses the ARM smc (secure monitor call) instruction to trigger
> +a mailbox-connected activity in firmware, executing on the very same core
> +as the caller.

The binding defines both hvc and smc as being valid methods for the mailbox.

> By nature this operation is synchronous and this mailbox
> +provides no way for asynchronous messages to be delivered the other way
> +round, from firmware to the OS, but asynchronous notification could also
> +be supported. However the value of r0/w0/x0 the firmware returns after
> +the smc call is delivered as a received message to the mailbox framework,
> +so a synchronous communication can be established, for a asynchronous
> +notification, no value will be returned. The exact meaning of both the
> +action the mailbox triggers as well as the return value is defined by
> +their users and is not subject to this binding.
> +
> +One use case of this mailbox is the SCMI interface, which uses shared memory
> +to transfer commands and parameters, and a mailbox to trigger a function
> +call. This allows SoCs without a separate management processor (or when
> +such a processor is not available or used) to use this standardized
> +interface anyway.
> +
> +This binding describes no hardware, but establishes a firmware interface.
> +Upon receiving an SMC using one of the described SMC function identifiers,
> +the firmware is expected to trigger some mailbox connected functionality.
> +The communication follows the ARM SMC calling convention[1].
> +Firmware expects an SMC function identifier in r0 or w0. The supported
> +identifiers are passed from consumers, or listed in the the arm,func-ids
> +properties as described below. The firmware can return one value in
> +the first SMC result register, it is expected to be an error value,
> +which shall be propagated to the mailbox client.
> +
> +Any core which supports the SMC or HVC instruction can be used, as long as
> +a firmware component running in EL3 or EL2 is handling these calls.
> +
> +Mailbox Device Node:
> +====================
> +
> +This node is expected to be a child of the /firmware node.
> +
> +Required properties:
> +--------------------
> +- compatible:		Shall be "arm,smc-mbox"
> +- #mbox-cells		Shall be 1 - the index of the channel needed.
> +- arm,num-chans		The number of channels supported.
> +- method:		A string, either:
> +			"hvc": if the driver shall use an HVC call, or
> +			"smc": if the driver shall use an SMC call.
> +
> +Optional properties:
> +- arm,func-ids		An array of 32-bit values specifying the function
> +			IDs used by each mailbox channel. Those function IDs
> +			follow the ARM SMC calling convention standard [1].
> +			There is one identifier per channel and the number
> +			of supported channels is determined by the length
> +			of this array.
> +- interrupts		SPI interrupts may be listed for notification,
> +			each channel should use a dedicated interrupt
> +			line.

I would not go about defining a specific kind of interrupt, since SPI is
a GIC terminology, this firmware interface could be used in premise with
any parent interrupt controller, for which the concept of a SPI/PPI/SGI
may not be relevant.

> +
> +Example:
> +--------
> +
> +	sram@910000 {
> +		compatible = "mmio-sram";
> +		reg = <0x0 0x93f000 0x0 0x1000>;
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		ranges = <0 0x0 0x93f000 0x1000>;
> +
> +		cpu_scp_lpri: scp-shmem@0 {
> +			compatible = "arm,scmi-shmem";
> +			reg = <0x0 0x200>;
> +		};
> +
> +		cpu_scp_hpri: scp-shmem@200 {
> +			compatible = "arm,scmi-shmem";
> +			reg = <0x200 0x200>;
> +		};
> +	};
> +
> +	smc_mbox: mailbox {
> +		#mbox-cells = <1>;
> +		compatible = "arm,smc-mbox";
> +		method = "smc";
> +		arm,num-chans = <0x2>;
> +		/* Optional */
> +		arm,func-ids = <0xc20000fe>, <0xc20000ff>;
> +	};
> +
> +	firmware {
> +		scmi {
> +			compatible = "arm,scmi";
> +			mboxes = <&mailbox 0 &mailbox 1>;

It is visually nicer (and more consistent with yoyr arm,func-ids example
to use:
			mboxes = <&mailbox 0>, <&mailbox 1>;

> +			mbox-names = "tx", "rx";
> +			shmem = <&cpu_scp_lpri &cpu_scp_hpri>;

			shmem = <&cpu_scp_lpri>, <&cpu_scp_hpri>;

Other than those, LGTM!
Sudeep Holla June 3, 2019, 4:56 p.m. UTC | #2
On Mon, Jun 03, 2019 at 09:22:16AM -0700, Florian Fainelli wrote:
> On 6/3/19 1:30 AM, peng.fan@nxp.com wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > The ARM SMC mailbox binding describes a firmware interface to trigger
> > actions in software layers running in the EL2 or EL3 exception levels.
> > The term "ARM" here relates to the SMC instruction as part of the ARM
> > instruction set, not as a standard endorsed by ARM Ltd.
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >
> > V2:
> > Introduce interrupts as a property.
> >
> > V1:
> > arm,func-ids is still kept as an optional property, because there is no
> > defined SMC funciton id passed from SCMI. So in my test, I still use
> > arm,func-ids for ARM SIP service.
> >
> >  .../devicetree/bindings/mailbox/arm-smc.txt        | 101 +++++++++++++++++++++
> >  1 file changed, 101 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mailbox/arm-smc.txt
> >
> > diff --git a/Documentation/devicetree/bindings/mailbox/arm-smc.txt b/Documentation/devicetree/bindings/mailbox/arm-smc.txt
> > new file mode 100644
> > index 000000000000..401887118c09
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mailbox/arm-smc.txt
> > @@ -0,0 +1,101 @@

[...]

> > +Optional properties:
> > +- arm,func-ids		An array of 32-bit values specifying the function
> > +			IDs used by each mailbox channel. Those function IDs
> > +			follow the ARM SMC calling convention standard [1].
> > +			There is one identifier per channel and the number
> > +			of supported channels is determined by the length
> > +			of this array.
> > +- interrupts		SPI interrupts may be listed for notification,
> > +			each channel should use a dedicated interrupt
> > +			line.
>
> I would not go about defining a specific kind of interrupt, since SPI is
> a GIC terminology, this firmware interface could be used in premise with
> any parent interrupt controller, for which the concept of a SPI/PPI/SGI
> may not be relevant.
>

While I agree the binding document may not contain specifics, I still
don't see how to use SGI with this. Also note it's generally reserved
for OS future use(IPC) and using this for other than IPC may be bit
challenging IMO. It opens up lots of questions.

--
Regards,
Sudeep
Andre Przywara June 3, 2019, 5:18 p.m. UTC | #3
On Mon, 3 Jun 2019 17:56:51 +0100
Sudeep Holla <sudeep.holla@arm.com> wrote:

Hi,

> On Mon, Jun 03, 2019 at 09:22:16AM -0700, Florian Fainelli wrote:
> > On 6/3/19 1:30 AM, peng.fan@nxp.com wrote:  
> > > From: Peng Fan <peng.fan@nxp.com>
> > >
> > > The ARM SMC mailbox binding describes a firmware interface to trigger
> > > actions in software layers running in the EL2 or EL3 exception levels.
> > > The term "ARM" here relates to the SMC instruction as part of the ARM
> > > instruction set, not as a standard endorsed by ARM Ltd.
> > >
> > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > ---
> > >
> > > V2:
> > > Introduce interrupts as a property.
> > >
> > > V1:
> > > arm,func-ids is still kept as an optional property, because there is no
> > > defined SMC funciton id passed from SCMI. So in my test, I still use
> > > arm,func-ids for ARM SIP service.
> > >
> > >  .../devicetree/bindings/mailbox/arm-smc.txt        | 101 +++++++++++++++++++++
> > >  1 file changed, 101 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/mailbox/arm-smc.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/mailbox/arm-smc.txt b/Documentation/devicetree/bindings/mailbox/arm-smc.txt
> > > new file mode 100644
> > > index 000000000000..401887118c09
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/mailbox/arm-smc.txt
> > > @@ -0,0 +1,101 @@  
> 
> [...]
> 
> > > +Optional properties:
> > > +- arm,func-ids		An array of 32-bit values specifying the function
> > > +			IDs used by each mailbox channel. Those function IDs
> > > +			follow the ARM SMC calling convention standard [1].
> > > +			There is one identifier per channel and the number
> > > +			of supported channels is determined by the length
> > > +			of this array.
> > > +- interrupts		SPI interrupts may be listed for notification,
> > > +			each channel should use a dedicated interrupt
> > > +			line.  
> >
> > I would not go about defining a specific kind of interrupt, since SPI is
> > a GIC terminology, this firmware interface could be used in premise with
> > any parent interrupt controller, for which the concept of a SPI/PPI/SGI
> > may not be relevant.
> >  
> 
> While I agree the binding document may not contain specifics, I still
> don't see how to use SGI with this. Also note it's generally reserved
> for OS future use(IPC) and using this for other than IPC may be bit
> challenging IMO. It opens up lots of questions.

Well, a PPI might be possible to use, although it's somewhat dodgy to hijack the GIC's (re-)distributor from EL3 to write to GICD_ISPENDR<n>. Need to ask Marc about his feelings towards this. But it's definitely possible from a hypervisor to inject arbitrary interrupts into a guest.

But more importantly: is there any actual reason this needs to be a GIC interrupt? If I understand the code correctly, this could just be any interrupt, including one of an interrupt combiner or a GPIO chip. So why not just use the standard wording of: "exactly one interrupt specifier for each channel"?

By the way: Shouldn't new bindings use the YAML format instead?

Cheers,
Andre.
Florian Fainelli June 6, 2019, 2:51 a.m. UTC | #4
On 6/3/2019 10:18 AM, Andre Przywara wrote:
> On Mon, 3 Jun 2019 17:56:51 +0100
> Sudeep Holla <sudeep.holla@arm.com> wrote:
> 
> Hi,
> 
>> On Mon, Jun 03, 2019 at 09:22:16AM -0700, Florian Fainelli wrote:
>>> On 6/3/19 1:30 AM, peng.fan@nxp.com wrote:  
>>>> From: Peng Fan <peng.fan@nxp.com>
>>>>
>>>> The ARM SMC mailbox binding describes a firmware interface to trigger
>>>> actions in software layers running in the EL2 or EL3 exception levels.
>>>> The term "ARM" here relates to the SMC instruction as part of the ARM
>>>> instruction set, not as a standard endorsed by ARM Ltd.
>>>>
>>>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>>>> ---
>>>>
>>>> V2:
>>>> Introduce interrupts as a property.
>>>>
>>>> V1:
>>>> arm,func-ids is still kept as an optional property, because there is no
>>>> defined SMC funciton id passed from SCMI. So in my test, I still use
>>>> arm,func-ids for ARM SIP service.
>>>>
>>>>  .../devicetree/bindings/mailbox/arm-smc.txt        | 101 +++++++++++++++++++++
>>>>  1 file changed, 101 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/mailbox/arm-smc.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/mailbox/arm-smc.txt b/Documentation/devicetree/bindings/mailbox/arm-smc.txt
>>>> new file mode 100644
>>>> index 000000000000..401887118c09
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/mailbox/arm-smc.txt
>>>> @@ -0,0 +1,101 @@  
>>
>> [...]
>>
>>>> +Optional properties:
>>>> +- arm,func-ids		An array of 32-bit values specifying the function
>>>> +			IDs used by each mailbox channel. Those function IDs
>>>> +			follow the ARM SMC calling convention standard [1].
>>>> +			There is one identifier per channel and the number
>>>> +			of supported channels is determined by the length
>>>> +			of this array.
>>>> +- interrupts		SPI interrupts may be listed for notification,
>>>> +			each channel should use a dedicated interrupt
>>>> +			line.  
>>>
>>> I would not go about defining a specific kind of interrupt, since SPI is
>>> a GIC terminology, this firmware interface could be used in premise with
>>> any parent interrupt controller, for which the concept of a SPI/PPI/SGI
>>> may not be relevant.
>>>  
>>
>> While I agree the binding document may not contain specifics, I still
>> don't see how to use SGI with this. Also note it's generally reserved
>> for OS future use(IPC) and using this for other than IPC may be bit
>> challenging IMO. It opens up lots of questions.
> 
> Well, a PPI might be possible to use, although it's somewhat dodgy to hijack the GIC's (re-)distributor from EL3 to write to GICD_ISPENDR<n>. Need to ask Marc about his feelings towards this. But it's definitely possible from a hypervisor to inject arbitrary interrupts into a guest.
> 
> But more importantly: is there any actual reason this needs to be a GIC interrupt? If I understand the code correctly, this could just be any interrupt, including one of an interrupt combiner or a GPIO chip. So why not just use the standard wording of: "exactly one interrupt specifier for each channel"?

That was my point, I am not stuck on using an SGI, or PPI, or anything
(even if that's what we have been using at the moment), any interrupt
would/should do here so the wording should be exactly as you indicated.
Peng Fan June 6, 2019, 3:24 a.m. UTC | #5
> Subject: Re: [PATCH V2 1/2] DT: mailbox: add binding doc for the ARM SMC
> mailbox
> 
> On Mon, 3 Jun 2019 17:56:51 +0100
> Sudeep Holla <sudeep.holla@arm.com> wrote:
> 
> Hi,
> 
> > On Mon, Jun 03, 2019 at 09:22:16AM -0700, Florian Fainelli wrote:
> > > On 6/3/19 1:30 AM, peng.fan@nxp.com wrote:
> > > > From: Peng Fan <peng.fan@nxp.com>
> > > >
> > > > The ARM SMC mailbox binding describes a firmware interface to
> > > > trigger actions in software layers running in the EL2 or EL3 exception
> levels.
> > > > The term "ARM" here relates to the SMC instruction as part of the
> > > > ARM instruction set, not as a standard endorsed by ARM Ltd.
> > > >
> > > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > > ---
> > > >
> > > > V2:
> > > > Introduce interrupts as a property.
> > > >
> > > > V1:
> > > > arm,func-ids is still kept as an optional property, because there
> > > > is no defined SMC funciton id passed from SCMI. So in my test, I
> > > > still use arm,func-ids for ARM SIP service.
> > > >
> > > >  .../devicetree/bindings/mailbox/arm-smc.txt        | 101
> +++++++++++++++++++++
> > > >  1 file changed, 101 insertions(+)  create mode 100644
> > > > Documentation/devicetree/bindings/mailbox/arm-smc.txt
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/mailbox/arm-smc.txt
> > > > b/Documentation/devicetree/bindings/mailbox/arm-smc.txt
> > > > new file mode 100644
> > > > index 000000000000..401887118c09
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/mailbox/arm-smc.txt
> > > > @@ -0,0 +1,101 @@
> >
> > [...]
> >
> > > > +Optional properties:
> > > > +- arm,func-ids		An array of 32-bit values specifying the function
> > > > +			IDs used by each mailbox channel. Those function IDs
> > > > +			follow the ARM SMC calling convention standard [1].
> > > > +			There is one identifier per channel and the number
> > > > +			of supported channels is determined by the length
> > > > +			of this array.
> > > > +- interrupts		SPI interrupts may be listed for notification,
> > > > +			each channel should use a dedicated interrupt
> > > > +			line.
> > >
> > > I would not go about defining a specific kind of interrupt, since
> > > SPI is a GIC terminology, this firmware interface could be used in
> > > premise with any parent interrupt controller, for which the concept
> > > of a SPI/PPI/SGI may not be relevant.
> > >
> >
> > While I agree the binding document may not contain specifics, I still
> > don't see how to use SGI with this. Also note it's generally reserved
> > for OS future use(IPC) and using this for other than IPC may be bit
> > challenging IMO. It opens up lots of questions.
> 
> Well, a PPI might be possible to use, although it's somewhat dodgy to hijack
> the GIC's (re-)distributor from EL3 to write to GICD_ISPENDR<n>. Need to ask
> Marc about his feelings towards this. But it's definitely possible from a
> hypervisor to inject arbitrary interrupts into a guest.
> 
> But more importantly: is there any actual reason this needs to be a GIC
> interrupt? 

No. I just test ATF with SPI when I posting out this. Should not restrict to be GIC.

If I understand the code correctly, this could just be any interrupt,
> including one of an interrupt combiner or a GPIO chip. So why not just use the
> standard wording of: "exactly one interrupt specifier for each channel"?

Agree.

> 
> By the way: Shouldn't new bindings use the YAML format instead?

I'll convert to YAML in next version.

Thanks,
Peng.

> 
> Cheers,
> Andre.
Sudeep Holla June 20, 2019, 9:22 a.m. UTC | #6
On Mon, Jun 03, 2019 at 04:30:04PM +0800, peng.fan@nxp.com wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> The ARM SMC mailbox binding describes a firmware interface to trigger
> actions in software layers running in the EL2 or EL3 exception levels.
> The term "ARM" here relates to the SMC instruction as part of the ARM
> instruction set, not as a standard endorsed by ARM Ltd.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> 
> V2:
> Introduce interrupts as a property.
> 
> V1:
> arm,func-ids is still kept as an optional property, because there is no
> defined SMC funciton id passed from SCMI. So in my test, I still use
> arm,func-ids for ARM SIP service.
> 
>  .../devicetree/bindings/mailbox/arm-smc.txt        | 101 +++++++++++++++++++++
>  1 file changed, 101 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mailbox/arm-smc.txt
> 
> diff --git a/Documentation/devicetree/bindings/mailbox/arm-smc.txt b/Documentation/devicetree/bindings/mailbox/arm-smc.txt
> new file mode 100644
> index 000000000000..401887118c09
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mailbox/arm-smc.txt
> @@ -0,0 +1,101 @@
> +ARM SMC Mailbox Interface
> +=========================
> +
> +This mailbox uses the ARM smc (secure monitor call) instruction to trigger
> +a mailbox-connected activity in firmware, executing on the very same core
> +as the caller. By nature this operation is synchronous and this mailbox
> +provides no way for asynchronous messages to be delivered the other way
> +round, from firmware to the OS, but asynchronous notification could also
> +be supported. However the value of r0/w0/x0 the firmware returns after
> +the smc call is delivered as a received message to the mailbox framework,
> +so a synchronous communication can be established, for a asynchronous
> +notification, no value will be returned. The exact meaning of both the
> +action the mailbox triggers as well as the return value is defined by
> +their users and is not subject to this binding.
> +
> +One use case of this mailbox is the SCMI interface, which uses shared memory
> +to transfer commands and parameters, and a mailbox to trigger a function
> +call. This allows SoCs without a separate management processor (or when
> +such a processor is not available or used) to use this standardized
> +interface anyway.
> +
> +This binding describes no hardware, but establishes a firmware interface.
> +Upon receiving an SMC using one of the described SMC function identifiers,
> +the firmware is expected to trigger some mailbox connected functionality.
> +The communication follows the ARM SMC calling convention[1].
> +Firmware expects an SMC function identifier in r0 or w0. The supported
> +identifiers are passed from consumers, or listed in the the arm,func-ids
> +properties as described below. The firmware can return one value in
> +the first SMC result register, it is expected to be an error value,
> +which shall be propagated to the mailbox client.
> +
> +Any core which supports the SMC or HVC instruction can be used, as long as
> +a firmware component running in EL3 or EL2 is handling these calls.
> +
> +Mailbox Device Node:
> +====================
> +
> +This node is expected to be a child of the /firmware node.
> +
> +Required properties:
> +--------------------
> +- compatible:		Shall be "arm,smc-mbox"
> +- #mbox-cells		Shall be 1 - the index of the channel needed.
> +- arm,num-chans		The number of channels supported.
> +- method:		A string, either:
> +			"hvc": if the driver shall use an HVC call, or
> +			"smc": if the driver shall use an SMC call.
> +
> +Optional properties:
> +- arm,func-ids		An array of 32-bit values specifying the function
> +			IDs used by each mailbox channel. Those function IDs
> +			follow the ARM SMC calling convention standard [1].
> +			There is one identifier per channel and the number
> +			of supported channels is determined by the length
> +			of this array.
> +- interrupts		SPI interrupts may be listed for notification,
> +			each channel should use a dedicated interrupt
> +			line.
> +

I think SMC mailbox as mostly unidirectional/Tx only channel. And the
interrupts here as stated are for notifications, so I prefer to keep
them separate channel. I assume SMC call return indicates completion.
Or do you plan to use these interrupts as the indication for completion
of the command? I see in patch 2/2 the absence of IRQ is anyway dealt
the way I mention above.

Does it make sense or am I missing something here ?

--
Regards,
Sudeep
Andre Przywara June 20, 2019, 4:13 p.m. UTC | #7
On Thu, 20 Jun 2019 10:22:41 +0100
Sudeep Holla <sudeep.holla@arm.com> wrote:

> On Mon, Jun 03, 2019 at 04:30:04PM +0800, peng.fan@nxp.com wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> > 
> > The ARM SMC mailbox binding describes a firmware interface to trigger
> > actions in software layers running in the EL2 or EL3 exception levels.
> > The term "ARM" here relates to the SMC instruction as part of the ARM
> > instruction set, not as a standard endorsed by ARM Ltd.
> > 
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> > 
> > V2:
> > Introduce interrupts as a property.
> > 
> > V1:
> > arm,func-ids is still kept as an optional property, because there is no
> > defined SMC funciton id passed from SCMI. So in my test, I still use
> > arm,func-ids for ARM SIP service.
> > 
> >  .../devicetree/bindings/mailbox/arm-smc.txt        | 101 +++++++++++++++++++++
> >  1 file changed, 101 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mailbox/arm-smc.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/mailbox/arm-smc.txt b/Documentation/devicetree/bindings/mailbox/arm-smc.txt
> > new file mode 100644
> > index 000000000000..401887118c09
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mailbox/arm-smc.txt
> > @@ -0,0 +1,101 @@
> > +ARM SMC Mailbox Interface
> > +=========================
> > +
> > +This mailbox uses the ARM smc (secure monitor call) instruction to trigger
> > +a mailbox-connected activity in firmware, executing on the very same core
> > +as the caller. By nature this operation is synchronous and this mailbox
> > +provides no way for asynchronous messages to be delivered the other way
> > +round, from firmware to the OS, but asynchronous notification could also
> > +be supported. However the value of r0/w0/x0 the firmware returns after
> > +the smc call is delivered as a received message to the mailbox framework,
> > +so a synchronous communication can be established, for a asynchronous
> > +notification, no value will be returned. The exact meaning of both the
> > +action the mailbox triggers as well as the return value is defined by
> > +their users and is not subject to this binding.
> > +
> > +One use case of this mailbox is the SCMI interface, which uses shared memory
> > +to transfer commands and parameters, and a mailbox to trigger a function
> > +call. This allows SoCs without a separate management processor (or when
> > +such a processor is not available or used) to use this standardized
> > +interface anyway.
> > +
> > +This binding describes no hardware, but establishes a firmware interface.
> > +Upon receiving an SMC using one of the described SMC function identifiers,
> > +the firmware is expected to trigger some mailbox connected functionality.
> > +The communication follows the ARM SMC calling convention[1].
> > +Firmware expects an SMC function identifier in r0 or w0. The supported
> > +identifiers are passed from consumers, or listed in the the arm,func-ids
> > +properties as described below. The firmware can return one value in
> > +the first SMC result register, it is expected to be an error value,
> > +which shall be propagated to the mailbox client.
> > +
> > +Any core which supports the SMC or HVC instruction can be used, as long as
> > +a firmware component running in EL3 or EL2 is handling these calls.
> > +
> > +Mailbox Device Node:
> > +====================
> > +
> > +This node is expected to be a child of the /firmware node.
> > +
> > +Required properties:
> > +--------------------
> > +- compatible:		Shall be "arm,smc-mbox"
> > +- #mbox-cells		Shall be 1 - the index of the channel needed.
> > +- arm,num-chans		The number of channels supported.
> > +- method:		A string, either:
> > +			"hvc": if the driver shall use an HVC call, or
> > +			"smc": if the driver shall use an SMC call.
> > +
> > +Optional properties:
> > +- arm,func-ids		An array of 32-bit values specifying the function
> > +			IDs used by each mailbox channel. Those function IDs
> > +			follow the ARM SMC calling convention standard [1].
> > +			There is one identifier per channel and the number
> > +			of supported channels is determined by the length
> > +			of this array.
> > +- interrupts		SPI interrupts may be listed for notification,
> > +			each channel should use a dedicated interrupt
> > +			line.
> > +  
> 
> I think SMC mailbox as mostly unidirectional/Tx only channel. And the
> interrupts here as stated are for notifications, so I prefer to keep
> them separate channel. I assume SMC call return indicates completion.
> Or do you plan to use these interrupts as the indication for completion
> of the command? I see in patch 2/2 the absence of IRQ is anyway dealt
> the way I mention above.
> 
> Does it make sense or am I missing something here ?

I think you are right. From a mailbox point of view "completion" means
that the trigger has reached the other side. A returning smc call is a
perfect indication of this fact. Whether the action triggered by this
mailbox command has completed is a totally separate question and out of
the scope of the mailbox. This should be handled by a higher level
protocol (SCPI in this case). Which could mean that this employs a
separate return mailbox channel, which is RX only and implemented by
interrupts. Which could or could not be part of this driver.

Cheers,
Andre
Jassi Brar June 20, 2019, 4:27 p.m. UTC | #8
On Thu, Jun 20, 2019 at 11:13 AM Andre Przywara <andre.przywara@arm.com> wrote:
>
> On Thu, 20 Jun 2019 10:22:41 +0100
> Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> > On Mon, Jun 03, 2019 at 04:30:04PM +0800, peng.fan@nxp.com wrote:
> > > From: Peng Fan <peng.fan@nxp.com>
> > >
> > > The ARM SMC mailbox binding describes a firmware interface to trigger
> > > actions in software layers running in the EL2 or EL3 exception levels.
> > > The term "ARM" here relates to the SMC instruction as part of the ARM
> > > instruction set, not as a standard endorsed by ARM Ltd.
> > >
> > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > ---
> > >
> > > V2:
> > > Introduce interrupts as a property.
> > >
> > > V1:
> > > arm,func-ids is still kept as an optional property, because there is no
> > > defined SMC funciton id passed from SCMI. So in my test, I still use
> > > arm,func-ids for ARM SIP service.
> > >
> > >  .../devicetree/bindings/mailbox/arm-smc.txt        | 101 +++++++++++++++++++++
> > >  1 file changed, 101 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/mailbox/arm-smc.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/mailbox/arm-smc.txt b/Documentation/devicetree/bindings/mailbox/arm-smc.txt
> > > new file mode 100644
> > > index 000000000000..401887118c09
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/mailbox/arm-smc.txt
> > > @@ -0,0 +1,101 @@
> > > +ARM SMC Mailbox Interface
> > > +=========================
> > > +
> > > +This mailbox uses the ARM smc (secure monitor call) instruction to trigger
> > > +a mailbox-connected activity in firmware, executing on the very same core
> > > +as the caller. By nature this operation is synchronous and this mailbox
> > > +provides no way for asynchronous messages to be delivered the other way
> > > +round, from firmware to the OS, but asynchronous notification could also
> > > +be supported. However the value of r0/w0/x0 the firmware returns after
> > > +the smc call is delivered as a received message to the mailbox framework,
> > > +so a synchronous communication can be established, for a asynchronous
> > > +notification, no value will be returned. The exact meaning of both the
> > > +action the mailbox triggers as well as the return value is defined by
> > > +their users and is not subject to this binding.
> > > +
> > > +One use case of this mailbox is the SCMI interface, which uses shared memory
> > > +to transfer commands and parameters, and a mailbox to trigger a function
> > > +call. This allows SoCs without a separate management processor (or when
> > > +such a processor is not available or used) to use this standardized
> > > +interface anyway.
> > > +
> > > +This binding describes no hardware, but establishes a firmware interface.
> > > +Upon receiving an SMC using one of the described SMC function identifiers,
> > > +the firmware is expected to trigger some mailbox connected functionality.
> > > +The communication follows the ARM SMC calling convention[1].
> > > +Firmware expects an SMC function identifier in r0 or w0. The supported
> > > +identifiers are passed from consumers, or listed in the the arm,func-ids
> > > +properties as described below. The firmware can return one value in
> > > +the first SMC result register, it is expected to be an error value,
> > > +which shall be propagated to the mailbox client.
> > > +
> > > +Any core which supports the SMC or HVC instruction can be used, as long as
> > > +a firmware component running in EL3 or EL2 is handling these calls.
> > > +
> > > +Mailbox Device Node:
> > > +====================
> > > +
> > > +This node is expected to be a child of the /firmware node.
> > > +
> > > +Required properties:
> > > +--------------------
> > > +- compatible:              Shall be "arm,smc-mbox"
> > > +- #mbox-cells              Shall be 1 - the index of the channel needed.
> > > +- arm,num-chans            The number of channels supported.
> > > +- method:          A string, either:
> > > +                   "hvc": if the driver shall use an HVC call, or
> > > +                   "smc": if the driver shall use an SMC call.
> > > +
> > > +Optional properties:
> > > +- arm,func-ids             An array of 32-bit values specifying the function
> > > +                   IDs used by each mailbox channel. Those function IDs
> > > +                   follow the ARM SMC calling convention standard [1].
> > > +                   There is one identifier per channel and the number
> > > +                   of supported channels is determined by the length
> > > +                   of this array.
> > > +- interrupts               SPI interrupts may be listed for notification,
> > > +                   each channel should use a dedicated interrupt
> > > +                   line.
> > > +
> >
> > I think SMC mailbox as mostly unidirectional/Tx only channel. And the
> > interrupts here as stated are for notifications, so I prefer to keep
> > them separate channel. I assume SMC call return indicates completion.
> > Or do you plan to use these interrupts as the indication for completion
> > of the command? I see in patch 2/2 the absence of IRQ is anyway dealt
> > the way I mention above.
> >
> > Does it make sense or am I missing something here ?
>
> I think you are right. From a mailbox point of view "completion" means
> that the trigger has reached the other side. A returning smc call is a
> perfect indication of this fact.
>
Yes. mailbox only cares about message delivery.

> Whether the action triggered by this
> mailbox command has completed is a totally separate question and out of
> the scope of the mailbox.
>
Yes, whether the message is accepted/rejected at protocol level is a
matter of upper layer (protocol).

> This should be handled by a higher level
> protocol (SCPI in this case). Which could mean that this employs a
> separate return mailbox channel, which is RX only and implemented by
> interrupts. Which could or could not be part of this driver.
>
Any message received over the same class of channel should be handled
in this driver.

Cheers
Rob Herring July 8, 2019, 10:19 p.m. UTC | #9
On Mon, Jun 03, 2019 at 04:30:04PM +0800, peng.fan@nxp.com wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> The ARM SMC mailbox binding describes a firmware interface to trigger
> actions in software layers running in the EL2 or EL3 exception levels.
> The term "ARM" here relates to the SMC instruction as part of the ARM
> instruction set, not as a standard endorsed by ARM Ltd.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> 
> V2:
> Introduce interrupts as a property.
> 
> V1:
> arm,func-ids is still kept as an optional property, because there is no
> defined SMC funciton id passed from SCMI. So in my test, I still use
> arm,func-ids for ARM SIP service.
> 
>  .../devicetree/bindings/mailbox/arm-smc.txt        | 101 +++++++++++++++++++++
>  1 file changed, 101 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mailbox/arm-smc.txt
> 
> diff --git a/Documentation/devicetree/bindings/mailbox/arm-smc.txt b/Documentation/devicetree/bindings/mailbox/arm-smc.txt
> new file mode 100644
> index 000000000000..401887118c09
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mailbox/arm-smc.txt
> @@ -0,0 +1,101 @@
> +ARM SMC Mailbox Interface
> +=========================
> +
> +This mailbox uses the ARM smc (secure monitor call) instruction to trigger
> +a mailbox-connected activity in firmware, executing on the very same core
> +as the caller. By nature this operation is synchronous and this mailbox
> +provides no way for asynchronous messages to be delivered the other way
> +round, from firmware to the OS, but asynchronous notification could also
> +be supported. However the value of r0/w0/x0 the firmware returns after
> +the smc call is delivered as a received message to the mailbox framework,
> +so a synchronous communication can be established, for a asynchronous
> +notification, no value will be returned. The exact meaning of both the
> +action the mailbox triggers as well as the return value is defined by
> +their users and is not subject to this binding.
> +
> +One use case of this mailbox is the SCMI interface, which uses shared memory
> +to transfer commands and parameters, and a mailbox to trigger a function
> +call. This allows SoCs without a separate management processor (or when
> +such a processor is not available or used) to use this standardized
> +interface anyway.
> +
> +This binding describes no hardware, but establishes a firmware interface.
> +Upon receiving an SMC using one of the described SMC function identifiers,
> +the firmware is expected to trigger some mailbox connected functionality.
> +The communication follows the ARM SMC calling convention[1].
> +Firmware expects an SMC function identifier in r0 or w0. The supported
> +identifiers are passed from consumers, or listed in the the arm,func-ids
> +properties as described below. The firmware can return one value in
> +the first SMC result register, it is expected to be an error value,
> +which shall be propagated to the mailbox client.
> +
> +Any core which supports the SMC or HVC instruction can be used, as long as
> +a firmware component running in EL3 or EL2 is handling these calls.
> +
> +Mailbox Device Node:
> +====================
> +
> +This node is expected to be a child of the /firmware node.
> +
> +Required properties:
> +--------------------
> +- compatible:		Shall be "arm,smc-mbox"
> +- #mbox-cells		Shall be 1 - the index of the channel needed.
> +- arm,num-chans		The number of channels supported.
> +- method:		A string, either:
> +			"hvc": if the driver shall use an HVC call, or
> +			"smc": if the driver shall use an SMC call.
> +
> +Optional properties:
> +- arm,func-ids		An array of 32-bit values specifying the function
> +			IDs used by each mailbox channel. Those function IDs
> +			follow the ARM SMC calling convention standard [1].
> +			There is one identifier per channel and the number
> +			of supported channels is determined by the length
> +			of this array.
> +- interrupts		SPI interrupts may be listed for notification,
> +			each channel should use a dedicated interrupt
> +			line.
> +
> +Example:
> +--------
> +
> +	sram@910000 {
> +		compatible = "mmio-sram";
> +		reg = <0x0 0x93f000 0x0 0x1000>;
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		ranges = <0 0x0 0x93f000 0x1000>;
> +
> +		cpu_scp_lpri: scp-shmem@0 {
> +			compatible = "arm,scmi-shmem";
> +			reg = <0x0 0x200>;
> +		};
> +
> +		cpu_scp_hpri: scp-shmem@200 {
> +			compatible = "arm,scmi-shmem";
> +			reg = <0x200 0x200>;
> +		};
> +	};
> +
> +	smc_mbox: mailbox {

This should be a child of 'firmware' node at least and really a child of 
the firmware component that implements the feature.

> +		#mbox-cells = <1>;
> +		compatible = "arm,smc-mbox";
> +		method = "smc";
> +		arm,num-chans = <0x2>;
> +		/* Optional */
> +		arm,func-ids = <0xc20000fe>, <0xc20000ff>;
> +	};
> +
> +	firmware {
> +		scmi {
> +			compatible = "arm,scmi";
> +			mboxes = <&mailbox 0 &mailbox 1>;
> +			mbox-names = "tx", "rx";
> +			shmem = <&cpu_scp_lpri &cpu_scp_hpri>;
> +		};
> +	};
> +
> +
> +[1]
> +http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0028a/index.html
> -- 
> 2.16.4
>
Peng Fan July 9, 2019, 1:40 a.m. UTC | #10
Hi Rob,

> Subject: Re: [PATCH V2 1/2] DT: mailbox: add binding doc for the ARM SMC
> mailbox
> 
> On Mon, Jun 03, 2019 at 04:30:04PM +0800, peng.fan@nxp.com wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > The ARM SMC mailbox binding describes a firmware interface to trigger
> > actions in software layers running in the EL2 or EL3 exception levels.
> > The term "ARM" here relates to the SMC instruction as part of the ARM
> > instruction set, not as a standard endorsed by ARM Ltd.
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >
> > V2:
> > Introduce interrupts as a property.
> >
> > V1:
> > arm,func-ids is still kept as an optional property, because there is
> > no defined SMC funciton id passed from SCMI. So in my test, I still
> > use arm,func-ids for ARM SIP service.
> >
> >  .../devicetree/bindings/mailbox/arm-smc.txt        | 101
> +++++++++++++++++++++
> >  1 file changed, 101 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/mailbox/arm-smc.txt
> >
> > diff --git a/Documentation/devicetree/bindings/mailbox/arm-smc.txt
> > b/Documentation/devicetree/bindings/mailbox/arm-smc.txt
> > new file mode 100644
> > index 000000000000..401887118c09
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mailbox/arm-smc.txt
> > @@ -0,0 +1,101 @@
> > +ARM SMC Mailbox Interface
> > +=========================
> > +
> > +This mailbox uses the ARM smc (secure monitor call) instruction to
> > +trigger a mailbox-connected activity in firmware, executing on the
> > +very same core as the caller. By nature this operation is synchronous
> > +and this mailbox provides no way for asynchronous messages to be
> > +delivered the other way round, from firmware to the OS, but
> > +asynchronous notification could also be supported. However the value
> > +of r0/w0/x0 the firmware returns after the smc call is delivered as a
> > +received message to the mailbox framework, so a synchronous
> > +communication can be established, for a asynchronous notification, no
> > +value will be returned. The exact meaning of both the action the
> > +mailbox triggers as well as the return value is defined by their users and is
> not subject to this binding.
> > +
> > +One use case of this mailbox is the SCMI interface, which uses shared
> > +memory to transfer commands and parameters, and a mailbox to trigger
> > +a function call. This allows SoCs without a separate management
> > +processor (or when such a processor is not available or used) to use
> > +this standardized interface anyway.
> > +
> > +This binding describes no hardware, but establishes a firmware interface.
> > +Upon receiving an SMC using one of the described SMC function
> > +identifiers, the firmware is expected to trigger some mailbox connected
> functionality.
> > +The communication follows the ARM SMC calling convention[1].
> > +Firmware expects an SMC function identifier in r0 or w0. The
> > +supported identifiers are passed from consumers, or listed in the the
> > +arm,func-ids properties as described below. The firmware can return
> > +one value in the first SMC result register, it is expected to be an
> > +error value, which shall be propagated to the mailbox client.
> > +
> > +Any core which supports the SMC or HVC instruction can be used, as
> > +long as a firmware component running in EL3 or EL2 is handling these calls.
> > +
> > +Mailbox Device Node:
> > +====================
> > +
> > +This node is expected to be a child of the /firmware node.
> > +
> > +Required properties:
> > +--------------------
> > +- compatible:		Shall be "arm,smc-mbox"
> > +- #mbox-cells		Shall be 1 - the index of the channel needed.
> > +- arm,num-chans		The number of channels supported.
> > +- method:		A string, either:
> > +			"hvc": if the driver shall use an HVC call, or
> > +			"smc": if the driver shall use an SMC call.
> > +
> > +Optional properties:
> > +- arm,func-ids		An array of 32-bit values specifying the function
> > +			IDs used by each mailbox channel. Those function IDs
> > +			follow the ARM SMC calling convention standard [1].
> > +			There is one identifier per channel and the number
> > +			of supported channels is determined by the length
> > +			of this array.
> > +- interrupts		SPI interrupts may be listed for notification,
> > +			each channel should use a dedicated interrupt
> > +			line.
> > +
> > +Example:
> > +--------
> > +
> > +	sram@910000 {
> > +		compatible = "mmio-sram";
> > +		reg = <0x0 0x93f000 0x0 0x1000>;
> > +		#address-cells = <1>;
> > +		#size-cells = <1>;
> > +		ranges = <0 0x0 0x93f000 0x1000>;
> > +
> > +		cpu_scp_lpri: scp-shmem@0 {
> > +			compatible = "arm,scmi-shmem";
> > +			reg = <0x0 0x200>;
> > +		};
> > +
> > +		cpu_scp_hpri: scp-shmem@200 {
> > +			compatible = "arm,scmi-shmem";
> > +			reg = <0x200 0x200>;
> > +		};
> > +	};
> > +
> > +	smc_mbox: mailbox {
> 
> This should be a child of 'firmware' node at least and really a child of the
> firmware component that implements the feature.

I checked other mbox driver, including the mbox used by ti sci, mbox used by
i.MX8QXP. both mbox dts node not a child a firmware node,
I am not sure why put mbox node into a child a firmware node here.

Thanks,
Peng.

> 
> > +		#mbox-cells = <1>;
> > +		compatible = "arm,smc-mbox";
> > +		method = "smc";
> > +		arm,num-chans = <0x2>;
> > +		/* Optional */
> > +		arm,func-ids = <0xc20000fe>, <0xc20000ff>;
> > +	};
> > +
> > +	firmware {
> > +		scmi {
> > +			compatible = "arm,scmi";
> > +			mboxes = <&mailbox 0 &mailbox 1>;
> > +			mbox-names = "tx", "rx";
> > +			shmem = <&cpu_scp_lpri &cpu_scp_hpri>;
> > +		};
> > +	};
> > +
> > +
> > +[1]
> > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Finfo
> >
> +center.arm.com%2Fhelp%2Findex.jsp%3Ftopic%3D%2Fcom.arm.doc.den002
> 8a%2
> >
> +Findex.html&amp;data=02%7C01%7Cpeng.fan%40nxp.com%7Cd8cf8b81b4f
> b49be5
> >
> +97c08d703f26576%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7
> C63698221
> >
> +1931902513&amp;sdata=RtHkNN07b%2FuzdJkiu0QujeJ6czrcwOwEI6Y6JW
> VpPkY%3D
> > +&amp;reserved=0
> > --
> > 2.16.4
> >
Rob Herring July 9, 2019, 1:31 p.m. UTC | #11
On Mon, Jul 8, 2019 at 7:40 PM Peng Fan <peng.fan@nxp.com> wrote:
>
> Hi Rob,
>
> > Subject: Re: [PATCH V2 1/2] DT: mailbox: add binding doc for the ARM SMC
> > mailbox
> >
> > On Mon, Jun 03, 2019 at 04:30:04PM +0800, peng.fan@nxp.com wrote:
> > > From: Peng Fan <peng.fan@nxp.com>
> > >
> > > The ARM SMC mailbox binding describes a firmware interface to trigger
> > > actions in software layers running in the EL2 or EL3 exception levels.
> > > The term "ARM" here relates to the SMC instruction as part of the ARM
> > > instruction set, not as a standard endorsed by ARM Ltd.
> > >
> > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > ---
> > >
> > > V2:
> > > Introduce interrupts as a property.
> > >
> > > V1:
> > > arm,func-ids is still kept as an optional property, because there is
> > > no defined SMC funciton id passed from SCMI. So in my test, I still
> > > use arm,func-ids for ARM SIP service.
> > >
> > >  .../devicetree/bindings/mailbox/arm-smc.txt        | 101
> > +++++++++++++++++++++
> > >  1 file changed, 101 insertions(+)
> > >  create mode 100644
> > > Documentation/devicetree/bindings/mailbox/arm-smc.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/mailbox/arm-smc.txt
> > > b/Documentation/devicetree/bindings/mailbox/arm-smc.txt
> > > new file mode 100644
> > > index 000000000000..401887118c09
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/mailbox/arm-smc.txt
> > > @@ -0,0 +1,101 @@
> > > +ARM SMC Mailbox Interface
> > > +=========================
> > > +
> > > +This mailbox uses the ARM smc (secure monitor call) instruction to
> > > +trigger a mailbox-connected activity in firmware, executing on the
> > > +very same core as the caller. By nature this operation is synchronous
> > > +and this mailbox provides no way for asynchronous messages to be
> > > +delivered the other way round, from firmware to the OS, but
> > > +asynchronous notification could also be supported. However the value
> > > +of r0/w0/x0 the firmware returns after the smc call is delivered as a
> > > +received message to the mailbox framework, so a synchronous
> > > +communication can be established, for a asynchronous notification, no
> > > +value will be returned. The exact meaning of both the action the
> > > +mailbox triggers as well as the return value is defined by their users and is
> > not subject to this binding.
> > > +
> > > +One use case of this mailbox is the SCMI interface, which uses shared
> > > +memory to transfer commands and parameters, and a mailbox to trigger
> > > +a function call. This allows SoCs without a separate management
> > > +processor (or when such a processor is not available or used) to use
> > > +this standardized interface anyway.
> > > +
> > > +This binding describes no hardware, but establishes a firmware interface.
> > > +Upon receiving an SMC using one of the described SMC function
> > > +identifiers, the firmware is expected to trigger some mailbox connected
> > functionality.
> > > +The communication follows the ARM SMC calling convention[1].
> > > +Firmware expects an SMC function identifier in r0 or w0. The
> > > +supported identifiers are passed from consumers, or listed in the the
> > > +arm,func-ids properties as described below. The firmware can return
> > > +one value in the first SMC result register, it is expected to be an
> > > +error value, which shall be propagated to the mailbox client.
> > > +
> > > +Any core which supports the SMC or HVC instruction can be used, as
> > > +long as a firmware component running in EL3 or EL2 is handling these calls.
> > > +
> > > +Mailbox Device Node:
> > > +====================
> > > +
> > > +This node is expected to be a child of the /firmware node.
> > > +
> > > +Required properties:
> > > +--------------------
> > > +- compatible:              Shall be "arm,smc-mbox"
> > > +- #mbox-cells              Shall be 1 - the index of the channel needed.
> > > +- arm,num-chans            The number of channels supported.
> > > +- method:          A string, either:
> > > +                   "hvc": if the driver shall use an HVC call, or
> > > +                   "smc": if the driver shall use an SMC call.
> > > +
> > > +Optional properties:
> > > +- arm,func-ids             An array of 32-bit values specifying the function
> > > +                   IDs used by each mailbox channel. Those function IDs
> > > +                   follow the ARM SMC calling convention standard [1].
> > > +                   There is one identifier per channel and the number
> > > +                   of supported channels is determined by the length
> > > +                   of this array.
> > > +- interrupts               SPI interrupts may be listed for notification,
> > > +                   each channel should use a dedicated interrupt
> > > +                   line.
> > > +
> > > +Example:
> > > +--------
> > > +
> > > +   sram@910000 {
> > > +           compatible = "mmio-sram";
> > > +           reg = <0x0 0x93f000 0x0 0x1000>;
> > > +           #address-cells = <1>;
> > > +           #size-cells = <1>;
> > > +           ranges = <0 0x0 0x93f000 0x1000>;
> > > +
> > > +           cpu_scp_lpri: scp-shmem@0 {
> > > +                   compatible = "arm,scmi-shmem";
> > > +                   reg = <0x0 0x200>;
> > > +           };
> > > +
> > > +           cpu_scp_hpri: scp-shmem@200 {
> > > +                   compatible = "arm,scmi-shmem";
> > > +                   reg = <0x200 0x200>;
> > > +           };
> > > +   };
> > > +
> > > +   smc_mbox: mailbox {
> >
> > This should be a child of 'firmware' node at least and really a child of the
> > firmware component that implements the feature.
>
> I checked other mbox driver, including the mbox used by ti sci, mbox used by
> i.MX8QXP. both mbox dts node not a child a firmware node,

Because those are actual h/w blocks and not implemented in firmware calls?

> I am not sure why put mbox node into a child a firmware node here.

If it is an interface provided by firmware, then it goes under /firmware.

Rob

Patch
diff mbox series

diff --git a/Documentation/devicetree/bindings/mailbox/arm-smc.txt b/Documentation/devicetree/bindings/mailbox/arm-smc.txt
new file mode 100644
index 000000000000..401887118c09
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/arm-smc.txt
@@ -0,0 +1,101 @@ 
+ARM SMC Mailbox Interface
+=========================
+
+This mailbox uses the ARM smc (secure monitor call) instruction to trigger
+a mailbox-connected activity in firmware, executing on the very same core
+as the caller. By nature this operation is synchronous and this mailbox
+provides no way for asynchronous messages to be delivered the other way
+round, from firmware to the OS, but asynchronous notification could also
+be supported. However the value of r0/w0/x0 the firmware returns after
+the smc call is delivered as a received message to the mailbox framework,
+so a synchronous communication can be established, for a asynchronous
+notification, no value will be returned. The exact meaning of both the
+action the mailbox triggers as well as the return value is defined by
+their users and is not subject to this binding.
+
+One use case of this mailbox is the SCMI interface, which uses shared memory
+to transfer commands and parameters, and a mailbox to trigger a function
+call. This allows SoCs without a separate management processor (or when
+such a processor is not available or used) to use this standardized
+interface anyway.
+
+This binding describes no hardware, but establishes a firmware interface.
+Upon receiving an SMC using one of the described SMC function identifiers,
+the firmware is expected to trigger some mailbox connected functionality.
+The communication follows the ARM SMC calling convention[1].
+Firmware expects an SMC function identifier in r0 or w0. The supported
+identifiers are passed from consumers, or listed in the the arm,func-ids
+properties as described below. The firmware can return one value in
+the first SMC result register, it is expected to be an error value,
+which shall be propagated to the mailbox client.
+
+Any core which supports the SMC or HVC instruction can be used, as long as
+a firmware component running in EL3 or EL2 is handling these calls.
+
+Mailbox Device Node:
+====================
+
+This node is expected to be a child of the /firmware node.
+
+Required properties:
+--------------------
+- compatible:		Shall be "arm,smc-mbox"
+- #mbox-cells		Shall be 1 - the index of the channel needed.
+- arm,num-chans		The number of channels supported.
+- method:		A string, either:
+			"hvc": if the driver shall use an HVC call, or
+			"smc": if the driver shall use an SMC call.
+
+Optional properties:
+- arm,func-ids		An array of 32-bit values specifying the function
+			IDs used by each mailbox channel. Those function IDs
+			follow the ARM SMC calling convention standard [1].
+			There is one identifier per channel and the number
+			of supported channels is determined by the length
+			of this array.
+- interrupts		SPI interrupts may be listed for notification,
+			each channel should use a dedicated interrupt
+			line.
+
+Example:
+--------
+
+	sram@910000 {
+		compatible = "mmio-sram";
+		reg = <0x0 0x93f000 0x0 0x1000>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges = <0 0x0 0x93f000 0x1000>;
+
+		cpu_scp_lpri: scp-shmem@0 {
+			compatible = "arm,scmi-shmem";
+			reg = <0x0 0x200>;
+		};
+
+		cpu_scp_hpri: scp-shmem@200 {
+			compatible = "arm,scmi-shmem";
+			reg = <0x200 0x200>;
+		};
+	};
+
+	smc_mbox: mailbox {
+		#mbox-cells = <1>;
+		compatible = "arm,smc-mbox";
+		method = "smc";
+		arm,num-chans = <0x2>;
+		/* Optional */
+		arm,func-ids = <0xc20000fe>, <0xc20000ff>;
+	};
+
+	firmware {
+		scmi {
+			compatible = "arm,scmi";
+			mboxes = <&mailbox 0 &mailbox 1>;
+			mbox-names = "tx", "rx";
+			shmem = <&cpu_scp_lpri &cpu_scp_hpri>;
+		};
+	};
+
+
+[1]
+http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0028a/index.html