On Wed, Jun 26, 2019 at 01:31:15PM +0000, Peng Fan wrote:

Hi All,

quoted

Subject: Re: [PATCH V2 2/2] mailbox: introduce ARM SMC based mailbox

On Tue, Jun 25, 2019 at 2:30 AM Peng Fan [off-list ref] wrote:

quoted

Hi Jassi

quoted

Subject: Re: [PATCH V2 2/2] mailbox: introduce ARM SMC based mailbox

On Mon, Jun 3, 2019 at 3:28 AM [off-list ref] wrote:

quoted

From: Peng Fan <peng.fan@nxp.com>

This mailbox driver implements a mailbox which signals transmitted
data via an ARM smc (secure monitor call) instruction. The mailbox
receiver is implemented in firmware and can synchronously return
data when it returns execution to the non-secure world again.
An asynchronous receive path is not implemented.
This allows the usage of a mailbox to trigger firmware actions on
SoCs which either don't have a separate management processor or on
which such a core is not available. A user of this mailbox could
be the SCP interface.

Modified from Andre Przywara's v2 patch https://lore
.kernel.org%2Fpatchwork%2Fpatch%2F812999%2F&amp;data=02%7C0

1%7

quoted

quoted

Cpeng.fa

quoted

n%40nxp.com%7C1237677cb01044ad714508d6f59f648f%7C686ea1d3bc2b4

quoted

quoted

c6fa92cd

quoted

99c5c301635%7C0%7C0%7C636966462272457978&sdata=Hzgeu43m5

quoted

quoted

ZkeRMtL8Bx

quoted

gUm3%2B6FBObib1OPHPlSccE%2B0%3D&reserved=0

Cc: Andre Przywara <andre.przywara@arm.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---

V2:
 Add interrupts notification support.

 drivers/mailbox/Kconfig                 |   7 ++
 drivers/mailbox/Makefile                |   2 +
 drivers/mailbox/arm-smc-mailbox.c       | 190

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

quoted

 include/linux/mailbox/arm-smc-mailbox.h |  10 ++
 4 files changed, 209 insertions(+)
 create mode 100644 drivers/mailbox/arm-smc-mailbox.c  create

mode

quoted

quoted

quoted

100644 include/linux/mailbox/arm-smc-mailbox.h

diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig index

595542bfae85..c3bd0f1ddcd8 100644

--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig

@@ -15,6 +15,13 @@ config ARM_MHU
          The controller has 3 mailbox channels, the last of which can

be

quoted

quoted

quoted

          used in Secure mode only.

+config ARM_SMC_MBOX
+       tristate "Generic ARM smc mailbox"
+       depends on OF && HAVE_ARM_SMCCC
+       help
+         Generic mailbox driver which uses ARM smc calls to call into
+         firmware for triggering mailboxes.
+
 config IMX_MBOX
        tristate "i.MX Mailbox"
        depends on ARCH_MXC || COMPILE_TEST diff --git
a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile index
c22fad6f696b..93918a84c91b 100644

--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile

@@ -7,6 +7,8 @@ obj-$(CONFIG_MAILBOX_TEST)      +=

mailbox-test.o

quoted

quoted

quoted

 obj-$(CONFIG_ARM_MHU)  += arm_mhu.o

+obj-$(CONFIG_ARM_SMC_MBOX)     += arm-smc-mailbox.o
+
 obj-$(CONFIG_IMX_MBOX) += imx-mailbox.o

 obj-$(CONFIG_ARMADA_37XX_RWTM_MBOX)    +=

armada-37xx-rwtm-mailbox.o

quoted

diff --git a/drivers/mailbox/arm-smc-mailbox.c

b/drivers/mailbox/arm-smc-mailbox.c
new file mode 100644
index 000000000000..fef6e38d8b98

--- /dev/null
+++ b/drivers/mailbox/arm-smc-mailbox.c

@@ -0,0 +1,190 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2016,2017 ARM Ltd.
+ * Copyright 2019 NXP
+ */
+
+#include <linux/arm-smccc.h>
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/interrupt.h>
+#include <linux/mailbox_controller.h> #include
+<linux/mailbox/arm-smc-mailbox.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+#define ARM_SMC_MBOX_USE_HVC   BIT(0)
+#define ARM_SMC_MBOX_USB_IRQ   BIT(1)
+

IRQ bit is unused (and unnecessary IMO)

quoted

+struct arm_smc_chan_data {
+       u32 function_id;
+       u32 flags;
+       int irq;
+};
+
+static int arm_smc_send_data(struct mbox_chan *link, void *data) {
+       struct arm_smc_chan_data *chan_data = link->con_priv;
+       struct arm_smccc_mbox_cmd *cmd = data;
+       struct arm_smccc_res res;
+       u32 function_id;
+
+       if (chan_data->function_id != UINT_MAX)
+               function_id = chan_data->function_id;
+       else
+               function_id = cmd->a0;
+

Not sure about chan_data->function_id.  Why restrict from DT?
'a0' is the function_id register, let the user pass func-id via the 'a0' like

other

quoted

quoted

values via 'a[1-7]'

Missed to reply this comment.

The firmware driver might not have func-id, such as SCMI/SCPI.
So add an optional func-id to let smc mailbox driver could
use smc SiP func id.

There is no end to conforming to protocols. Controller drivers should
be written having no particular client in mind.

If the func-id needs be passed from user, then the chan_id suggested
by Sudeep should also be passed from user, not in mailbox driver.

Why ? I understand SMC may have 1-1 mapping from DT to channel id, but
that may not be true for other mailbox controller. The client is provided
with mbox handle in DT and the mbox APIs are used to get the controller
handle. The client just uses the same and when it calls say send_data,
controller understands which channel the handle is mapped to and
client need not have remote idea on channel id. If by user you mean
DT yes but as described as it's indirect.

Jassi, so from your point, arm_smc_send_data just send a0 - a6
to firmware, right?

Sudeep, Andre, Florian,

What's your suggestion? SCMI not support, do you have
plan to add smc transport in SCMI?

I *wish* we could abstract all the transport protocol behind the mailbox
APIs and SCMI just deals with message protocol as it is a message protocol
specification.

--
Regards,
Sudeep