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 [off-list ref] wrote:

Hi,

quoted

On Mon, Jun 03, 2019 at 09:22:16AM -0700, Florian Fainelli wrote:

quoted

On 6/3/19 1:30 AM, peng.fan@nxp.com wrote:

quoted

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.

quoted

quoted

quoted

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

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

quoted

quoted

quoted

 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 @@

[...]

quoted

quoted

+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.