[PATCH 0/2] firmware: arm_scmi: add smc/hvc transports support

[PATCH 0/2] firmware: arm_scmi: add smc/hvc transports support

[peng.fan]

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

This is to add smc/hvc transports support, based on Viresh's v6.
SCMI firmware could be implemented in EL3, S-EL1, NS-EL2 or other
A core exception level. Then smc/hvc could be used. And for vendor
specific firmware, a wrapper layer could added in EL3, S-EL1,
NS-EL2 and etc to translate SCMI calls to vendor specific firmware calls.

A new compatible string arm,scmi-smc is added. arm,scmi is still for
mailbox transports.

Each protocol could use its own smc/hvc id or share the same smc/hvc id
Each protocol could use its own shmem or share the same shmem
Per smc/hvc, only Tx supported.

Peng Fan (2):
  dt-bindings: arm: arm,scmi: add smc/hvc transports
  firmware: arm_scmi: add smc/hvc transports

 Documentation/devicetree/bindings/arm/arm,scmi.txt |   4 +-
 drivers/firmware/arm_scmi/Makefile                 |   2 +-
 drivers/firmware/arm_scmi/common.h                 |   1 +
 drivers/firmware/arm_scmi/driver.c                 |   1 +
 drivers/firmware/arm_scmi/smc.c                    | 177 +++++++++++++++++++++
 5 files changed, 183 insertions(+), 2 deletions(-)
 create mode 100644 drivers/firmware/arm_scmi/smc.c

-- 
2.16.4

[PATCH 1/2] dt-bindings: arm: arm,scmi: add smc/hvc transports

[peng.fan]

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

SCMI could use SMC/HVC as tranports, so add into devicetree
binding doc.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 Documentation/devicetree/bindings/arm/arm,scmi.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/arm/arm,scmi.txt b/Documentation/devicetree/bindings/arm/arm,scmi.txt
index f493d69e6194..03cff8b55a93 100644
--- a/Documentation/devicetree/bindings/arm/arm,scmi.txt
+++ b/Documentation/devicetree/bindings/arm/arm,scmi.txt
@@ -14,7 +14,7 @@ Required properties:
 
 The scmi node with the following properties shall be under the /firmware/ node.
 
-- compatible : shall be "arm,scmi"
+- compatible : shall be "arm,scmi" or "arm,scmi-smc"
 - mboxes: List of phandle and mailbox channel specifiers. It should contain
 	  exactly one or two mailboxes, one for transmitting messages("tx")
 	  and another optional for receiving the notifications("rx") if
@@ -25,6 +25,8 @@ The scmi node with the following properties shall be under the /firmware/ node.
 	  protocol identifier for a given sub-node.
 - #size-cells : should be '0' as 'reg' property doesn't have any size
 	  associated with it.
+- arm,smc-id : SMC id required when using smc transports
+- arm,hvc-id : HVC id required when using hvc transports
 
 Optional properties:
 
-- 
2.16.4

[PATCH 2/2] firmware: arm_scmi: add smc/hvc transports

[peng.fan]

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

Add SCMI smc/hvc transports support.

Each protocol could have its own smc/hvc id, or share the same id.
There is no Rx, only Tx because of smc/hvc not support Rx.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/firmware/arm_scmi/Makefile |   2 +-
 drivers/firmware/arm_scmi/common.h |   1 +
 drivers/firmware/arm_scmi/driver.c |   1 +
 drivers/firmware/arm_scmi/smc.c    | 177 +++++++++++++++++++++++++++++++++++++
 4 files changed, 180 insertions(+), 1 deletion(-)
 create mode 100644 drivers/firmware/arm_scmi/smc.c

diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile
index 6694d0d908d6..6b1b0d6c6d0e 100644
--- a/drivers/firmware/arm_scmi/Makefile
+++ b/drivers/firmware/arm_scmi/Makefile
@@ -2,6 +2,6 @@
 obj-y	= scmi-bus.o scmi-driver.o scmi-protocols.o scmi-transport.o
 scmi-bus-y = bus.o
 scmi-driver-y = driver.o
-scmi-transport-y = mailbox.o shmem.o
+scmi-transport-y = mailbox.o shmem.o smc.o
 scmi-protocols-y = base.o clock.o perf.o power.o reset.o sensors.o
 obj-$(CONFIG_ARM_SCMI_POWER_DOMAIN) += scmi_pm_domain.o
diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index 5df262a564a4..f9ff5cdb68be 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -207,6 +207,7 @@ struct scmi_desc {
 };
 
 extern const struct scmi_desc scmi_mailbox_desc;
+extern const struct scmi_desc scmi_smc_desc;
 
 void scmi_rx_callback(struct scmi_chan_info *cinfo, u32 msg_hdr);
 void scmi_free_channel(struct scmi_chan_info *cinfo, struct idr *idr, int id);
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index dbec767222e9..c759dd094374 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -827,6 +827,7 @@ ATTRIBUTE_GROUPS(versions);
 /* Each compatible listed below must have descriptor associated with it */
 static const struct of_device_id scmi_of_match[] = {
 	{ .compatible = "arm,scmi", .data = &scmi_mailbox_desc },
+	{ .compatible = "arm,scmi-smc", .data = &scmi_smc_desc },
 	{ /* Sentinel */ },
 };
 
diff --git a/drivers/firmware/arm_scmi/smc.c b/drivers/firmware/arm_scmi/smc.c
new file mode 100644
index 000000000000..4af0bbc4c827
--- /dev/null
+++ b/drivers/firmware/arm_scmi/smc.c
@@ -0,0 +1,177 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * System Control and Management Interface (SCMI) Message SMC/HVC
+ * Transport driver
+ *
+ * Copyright 2020 NXP
+ */
+
+#include <linux/arm-smccc.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/slab.h>
+
+#include "common.h"
+
+/**
+ * struct scmi_smc - Structure representing a SCMI smc transport
+ *
+ * @cinfo: SCMI channel info
+ * @shmem: Transmit/Receive shared memory area
+ * @func_id: smc/hvc call function id
+ * @smc: Indicate it is smc channel or hvc channel
+ */
+
+struct scmi_smc {
+	struct scmi_chan_info *cinfo;
+	struct scmi_shared_mem __iomem *shmem;
+	u32 func_id;
+	bool smc;
+};
+
+#define client_to_scmi_smc(c) container_of(c, struct scmi_smc, cl)
+
+static bool smc_chan_available(struct device *dev, int idx)
+{
+	return true;
+}
+
+static int smc_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
+			  bool tx)
+{
+	struct device *cdev = cinfo->dev;
+	struct scmi_smc *scmi_info;
+	struct device_node *shmem;
+	resource_size_t size;
+	struct resource res;
+	u32 func_id;
+	int ret;
+
+	if (!tx)
+		return -ENODEV;
+
+	scmi_info = devm_kzalloc(dev, sizeof(*scmi_info), GFP_KERNEL);
+	if (!scmi_info)
+		return -ENOMEM;
+
+	shmem = of_parse_phandle(cdev->of_node, "shmem", 0);
+	if (!shmem)
+		shmem = of_parse_phandle(dev->of_node, "shmem", 0);
+	ret = of_address_to_resource(shmem, 0, &res);
+	of_node_put(shmem);
+	if (ret) {
+		dev_err(cdev, "failed to get SCMI Tx shared memory\n");
+		return ret;
+	}
+
+	size = resource_size(&res);
+	scmi_info->shmem = devm_ioremap(dev, res.start, size);
+	if (!scmi_info->shmem) {
+		dev_err(dev, "failed to ioremap SCMI Tx shared memory\n");
+		return -EADDRNOTAVAIL;
+	}
+
+	ret = of_property_read_u32(cdev->of_node, "arm,smc-id", &func_id);
+	if (ret)
+		ret = of_property_read_u32(dev->of_node, "arm,smc-id",
+					   &func_id);
+	if (!ret) {
+		scmi_info->smc = true;
+	} else {
+		ret = of_property_read_u32(cdev->of_node, "arm,hvc-id",
+					   &func_id);
+		if (ret)
+			ret = of_property_read_u32(dev->of_node,
+						   "arm,hvc-id",
+						   &func_id);
+
+		if (ret) {
+			dev_err(dev, "failed to get SCMI arm,smc/hvc_id\n");
+			return ret;
+		}
+		scmi_info->smc = false;
+	}
+
+	scmi_info->func_id = func_id;
+	scmi_info->cinfo = cinfo;
+	cinfo->transport_info = scmi_info;
+
+	iowrite32(BIT(0), &scmi_info->shmem->channel_status);
+
+	return 0;
+}
+
+static int smc_chan_free(int id, void *p, void *data)
+{
+	struct scmi_chan_info *cinfo = p;
+	struct scmi_smc *scmi_info = cinfo->transport_info;
+
+	cinfo->transport_info = NULL;
+	scmi_info->cinfo = NULL;
+
+	scmi_free_channel(cinfo, data, id);
+
+	return 0;
+}
+
+static int smc_send_message(struct scmi_chan_info *cinfo,
+			    struct scmi_xfer *xfer)
+{
+	struct scmi_smc *scmi_info = cinfo->transport_info;
+	struct arm_smccc_res res;
+	int ret;
+
+	shmem_tx_prepare(scmi_info->shmem, xfer);
+
+	if (scmi_info->smc)
+		arm_smccc_smc(scmi_info->func_id, 0, 0, 0, 0, 0, 0, 0, &res);
+	else
+		arm_smccc_hvc(scmi_info->func_id, 0, 0, 0, 0, 0, 0, 0, &res);
+
+	ret = res.a0;
+	if (ret > 0)
+		ret = 0;
+
+	scmi_rx_callback(scmi_info->cinfo, shmem_read_header(scmi_info->shmem));
+
+	return ret;
+}
+
+static void smc_mark_txdone(struct scmi_chan_info *cinfo, int ret)
+{
+}
+
+static void smc_fetch_response(struct scmi_chan_info *cinfo,
+			       struct scmi_xfer *xfer)
+{
+	struct scmi_smc *scmi_info = cinfo->transport_info;
+
+	shmem_fetch_response(scmi_info->shmem, xfer);
+}
+
+static bool
+smc_poll_done(struct scmi_chan_info *cinfo, struct scmi_xfer *xfer)
+{
+	struct scmi_smc *scmi_info = cinfo->transport_info;
+
+	return shmem_poll_done(scmi_info->shmem, xfer);
+}
+
+static struct scmi_transport_ops scmi_smc_ops = {
+	.chan_available = smc_chan_available,
+	.chan_setup = smc_chan_setup,
+	.chan_free = smc_chan_free,
+	.send_message = smc_send_message,
+	.mark_txdone = smc_mark_txdone,
+	.fetch_response = smc_fetch_response,
+	.poll_done = smc_poll_done,
+};
+
+const struct scmi_desc scmi_smc_desc = {
+	.ops = &scmi_smc_ops,
+	.max_rx_timeout_ms = 30,
+	.max_msg = 1,
+	.max_msg_size = 128,
+};
-- 
2.16.4

Re: [PATCH 1/2] dt-bindings: arm: arm,scmi: add smc/hvc transports

[Rob Herring]

On Thu, Feb 06, 2020 at 09:01:25PM +0800, peng.fan@nxp.com wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> SCMI could use SMC/HVC as tranports, so add into devicetree
> binding doc.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  Documentation/devicetree/bindings/arm/arm,scmi.txt | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/arm/arm,scmi.txt b/Documentation/devicetree/bindings/arm/arm,scmi.txt
> index f493d69e6194..03cff8b55a93 100644
> --- a/Documentation/devicetree/bindings/arm/arm,scmi.txt
> +++ b/Documentation/devicetree/bindings/arm/arm,scmi.txt
> @@ -14,7 +14,7 @@ Required properties:
>  
>  The scmi node with the following properties shall be under the /firmware/ node.
>  
> -- compatible : shall be "arm,scmi"
> +- compatible : shall be "arm,scmi" or "arm,scmi-smc"
>  - mboxes: List of phandle and mailbox channel specifiers. It should contain
>  	  exactly one or two mailboxes, one for transmitting messages("tx")
>  	  and another optional for receiving the notifications("rx") if
> @@ -25,6 +25,8 @@ The scmi node with the following properties shall be under the /firmware/ node.
>  	  protocol identifier for a given sub-node.
>  - #size-cells : should be '0' as 'reg' property doesn't have any size
>  	  associated with it.
> +- arm,smc-id : SMC id required when using smc transports
> +- arm,hvc-id : HVC id required when using hvc transports

Don't the SMC ids get standardized?

>  
>  Optional properties:
>  
> -- 
> 2.16.4
> 

RE: [PATCH 1/2] dt-bindings: arm: arm,scmi: add smc/hvc transports

[Peng Fan]

> Subject: Re: [PATCH 1/2] dt-bindings: arm: arm,scmi: add smc/hvc transports
> 
> On Thu, Feb 06, 2020 at 09:01:25PM +0800, peng.fan@nxp.com wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > SCMI could use SMC/HVC as tranports, so add into devicetree binding
> > doc.
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >  Documentation/devicetree/bindings/arm/arm,scmi.txt | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > b/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > index f493d69e6194..03cff8b55a93 100644
> > --- a/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > +++ b/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > @@ -14,7 +14,7 @@ Required properties:
> >
> >  The scmi node with the following properties shall be under the /firmware/
> node.
> >
> > -- compatible : shall be "arm,scmi"
> > +- compatible : shall be "arm,scmi" or "arm,scmi-smc"
> >  - mboxes: List of phandle and mailbox channel specifiers. It should contain
> >  	  exactly one or two mailboxes, one for transmitting messages("tx")
> >  	  and another optional for receiving the notifications("rx") if @@
> > -25,6 +25,8 @@ The scmi node with the following properties shall be under
> the /firmware/ node.
> >  	  protocol identifier for a given sub-node.
> >  - #size-cells : should be '0' as 'reg' property doesn't have any size
> >  	  associated with it.
> > +- arm,smc-id : SMC id required when using smc transports
> > +- arm,hvc-id : HVC id required when using hvc transports
> 
> Don't the SMC ids get standardized?

For now, there is no standard SCMI SMC/HVC ID.

Sudeep, Is there any plan from ARM? Or how you think about this patch?

Thanks,
Peng.

> 
> >
> >  Optional properties:
> >
> > --
> > 2.16.4
> >

Re: [PATCH 1/2] dt-bindings: arm: arm,scmi: add smc/hvc transports

[Marc Zyngier]

On 2020-02-06 13:01, peng.fan@nxp.com wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> SCMI could use SMC/HVC as tranports, so add into devicetree
> binding doc.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  Documentation/devicetree/bindings/arm/arm,scmi.txt | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/arm/arm,scmi.txt
> b/Documentation/devicetree/bindings/arm/arm,scmi.txt
> index f493d69e6194..03cff8b55a93 100644
> --- a/Documentation/devicetree/bindings/arm/arm,scmi.txt
> +++ b/Documentation/devicetree/bindings/arm/arm,scmi.txt
> @@ -14,7 +14,7 @@ Required properties:
> 
>  The scmi node with the following properties shall be under the 
> /firmware/ node.
> 
> -- compatible : shall be "arm,scmi"
> +- compatible : shall be "arm,scmi" or "arm,scmi-smc"
>  - mboxes: List of phandle and mailbox channel specifiers. It should 
> contain
>  	  exactly one or two mailboxes, one for transmitting messages("tx")
>  	  and another optional for receiving the notifications("rx") if
> @@ -25,6 +25,8 @@ The scmi node with the following properties shall be
> under the /firmware/ node.
>  	  protocol identifier for a given sub-node.
>  - #size-cells : should be '0' as 'reg' property doesn't have any size
>  	  associated with it.
> +- arm,smc-id : SMC id required when using smc transports
> +- arm,hvc-id : HVC id required when using hvc transports
> 
>  Optional properties:

Not directly related to DT: Why do we need to distinguish between SMC 
and HVC?
Other SMC/HVC capable protocols are able to pick the right one based on 
the PSCI
conduit.

This is how the Spectre mitigations work already. Why is that any 
different?

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH 1/2] dt-bindings: arm: arm,scmi: add smc/hvc transports

[Sudeep Holla]

On Fri, Feb 07, 2020 at 10:08:36AM +0000, Marc Zyngier wrote:
> On 2020-02-06 13:01, peng.fan@nxp.com wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > SCMI could use SMC/HVC as tranports, so add into devicetree
> > binding doc.
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >  Documentation/devicetree/bindings/arm/arm,scmi.txt | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > b/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > index f493d69e6194..03cff8b55a93 100644
> > --- a/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > +++ b/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > @@ -14,7 +14,7 @@ Required properties:
> >
> >  The scmi node with the following properties shall be under the
> > /firmware/ node.
> >
> > -- compatible : shall be "arm,scmi"
> > +- compatible : shall be "arm,scmi" or "arm,scmi-smc"
> >  - mboxes: List of phandle and mailbox channel specifiers. It should
> > contain
> >  	  exactly one or two mailboxes, one for transmitting messages("tx")
> >  	  and another optional for receiving the notifications("rx") if
> > @@ -25,6 +25,8 @@ The scmi node with the following properties shall be
> > under the /firmware/ node.
> >  	  protocol identifier for a given sub-node.
> >  - #size-cells : should be '0' as 'reg' property doesn't have any size
> >  	  associated with it.
> > +- arm,smc-id : SMC id required when using smc transports
> > +- arm,hvc-id : HVC id required when using hvc transports
> >
> >  Optional properties:
>
> Not directly related to DT: Why do we need to distinguish between SMC and
> HVC?

IIUC you want just one property to get the function ID ? Does that align
with what you are saying ? I wanted to ask the same question and I see
no need for 2 different properties.

> Other SMC/HVC capable protocols are able to pick the right one based on the
> PSCI conduit.
>

This make it clear, but I am asking to  be sure.

> This is how the Spectre mitigations work already. Why is that any different?
>

I don't see any need for it to be different.

--
Regards,
Sudeep

Re: [PATCH 1/2] dt-bindings: arm: arm,scmi: add smc/hvc transports

[Marc Zyngier]

On 2020-02-07 10:47, Sudeep Holla wrote:
> On Fri, Feb 07, 2020 at 10:08:36AM +0000, Marc Zyngier wrote:
>> On 2020-02-06 13:01, peng.fan@nxp.com wrote:
>> > From: Peng Fan <peng.fan@nxp.com>
>> >
>> > SCMI could use SMC/HVC as tranports, so add into devicetree
>> > binding doc.
>> >
>> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
>> > ---
>> >  Documentation/devicetree/bindings/arm/arm,scmi.txt | 4 +++-
>> >  1 file changed, 3 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/Documentation/devicetree/bindings/arm/arm,scmi.txt
>> > b/Documentation/devicetree/bindings/arm/arm,scmi.txt
>> > index f493d69e6194..03cff8b55a93 100644
>> > --- a/Documentation/devicetree/bindings/arm/arm,scmi.txt
>> > +++ b/Documentation/devicetree/bindings/arm/arm,scmi.txt
>> > @@ -14,7 +14,7 @@ Required properties:
>> >
>> >  The scmi node with the following properties shall be under the
>> > /firmware/ node.
>> >
>> > -- compatible : shall be "arm,scmi"
>> > +- compatible : shall be "arm,scmi" or "arm,scmi-smc"
>> >  - mboxes: List of phandle and mailbox channel specifiers. It should
>> > contain
>> >  	  exactly one or two mailboxes, one for transmitting messages("tx")
>> >  	  and another optional for receiving the notifications("rx") if
>> > @@ -25,6 +25,8 @@ The scmi node with the following properties shall be
>> > under the /firmware/ node.
>> >  	  protocol identifier for a given sub-node.
>> >  - #size-cells : should be '0' as 'reg' property doesn't have any size
>> >  	  associated with it.
>> > +- arm,smc-id : SMC id required when using smc transports
>> > +- arm,hvc-id : HVC id required when using hvc transports
>> >
>> >  Optional properties:
>> 
>> Not directly related to DT: Why do we need to distinguish between SMC 
>> and
>> HVC?
> 
> IIUC you want just one property to get the function ID ? Does that 
> align
> with what you are saying ? I wanted to ask the same question and I see
> no need for 2 different properties.

Exactly. Using SMC or HVC should come from the context, and there is 
zero
value in having different different IDs, depending on the conduit.

We *really* want SMC and HVC to behave the same way. Any attempt to make
them different should just be NAKed.

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...

RE: [PATCH 1/2] dt-bindings: arm: arm,scmi: add smc/hvc transports

[Peng Fan]

> Subject: Re: [PATCH 1/2] dt-bindings: arm: arm,scmi: add smc/hvc transports
> 
> On Fri, Feb 07, 2020 at 10:08:36AM +0000, Marc Zyngier wrote:
> > On 2020-02-06 13:01, peng.fan@nxp.com wrote:
> > > From: Peng Fan <peng.fan@nxp.com>
> > >
> > > SCMI could use SMC/HVC as tranports, so add into devicetree binding
> > > doc.
> > >
> > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > ---
> > >  Documentation/devicetree/bindings/arm/arm,scmi.txt | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > > b/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > > index f493d69e6194..03cff8b55a93 100644
> > > --- a/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > > +++ b/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > > @@ -14,7 +14,7 @@ Required properties:
> > >
> > >  The scmi node with the following properties shall be under the
> > > /firmware/ node.
> > >
> > > -- compatible : shall be "arm,scmi"
> > > +- compatible : shall be "arm,scmi" or "arm,scmi-smc"
> > >  - mboxes: List of phandle and mailbox channel specifiers. It should
> > > contain
> > >  	  exactly one or two mailboxes, one for transmitting messages("tx")
> > >  	  and another optional for receiving the notifications("rx") if @@
> > > -25,6 +25,8 @@ The scmi node with the following properties shall be
> > > under the /firmware/ node.
> > >  	  protocol identifier for a given sub-node.
> > >  - #size-cells : should be '0' as 'reg' property doesn't have any size
> > >  	  associated with it.
> > > +- arm,smc-id : SMC id required when using smc transports
> > > +- arm,hvc-id : HVC id required when using hvc transports
> > >
> > >  Optional properties:
> >
> > Not directly related to DT: Why do we need to distinguish between SMC
> > and HVC?
> 
> IIUC you want just one property to get the function ID ? Does that align with
> what you are saying ? I wanted to ask the same question and I see no need for
> 2 different properties.

The multiple protocols might use SMC or HVC. Saying

 Protocol@x {
    method="smc";
    arm,func-id=<0x....>
 };
 Protocol@y {
    method="hvc";
    arm,func-id=<0x....>
 };

With my propose:

Protocol@x {
    arm,smc-id=<0x....>
 };
 Protocol@y {
    arm,hvc-id=<0x....>
 };

No need an extra method property to indicate it is smc or hvc.
The driver use take arm,smc-id as SMC, arm,hvc-id as HVC.

Thanks,
Peng.

> 
> > Other SMC/HVC capable protocols are able to pick the right one based
> > on the PSCI conduit.
> >
> 
> This make it clear, but I am asking to  be sure.
> 
> > This is how the Spectre mitigations work already. Why is that any different?
> >
> 
> I don't see any need for it to be different.
> 
> --
> Regards,
> Sudeep

Re: [PATCH 1/2] dt-bindings: arm: arm,scmi: add smc/hvc transports

[Sudeep Holla]

On Thu, Feb 06, 2020 at 02:59:47PM -0700, Rob Herring wrote:
> On Thu, Feb 06, 2020 at 09:01:25PM +0800, peng.fan@nxp.com wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > SCMI could use SMC/HVC as tranports, so add into devicetree
> > binding doc.
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >  Documentation/devicetree/bindings/arm/arm,scmi.txt | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/arm/arm,scmi.txt b/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > index f493d69e6194..03cff8b55a93 100644
> > --- a/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > +++ b/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > @@ -14,7 +14,7 @@ Required properties:
> >
> >  The scmi node with the following properties shall be under the /firmware/ node.
> >
> > -- compatible : shall be "arm,scmi"
> > +- compatible : shall be "arm,scmi" or "arm,scmi-smc"
> >  - mboxes: List of phandle and mailbox channel specifiers. It should contain
> >  	  exactly one or two mailboxes, one for transmitting messages("tx")
> >  	  and another optional for receiving the notifications("rx") if
> > @@ -25,6 +25,8 @@ The scmi node with the following properties shall be under the /firmware/ node.
> >  	  protocol identifier for a given sub-node.
> >  - #size-cells : should be '0' as 'reg' property doesn't have any size
> >  	  associated with it.
> > +- arm,smc-id : SMC id required when using smc transports
> > +- arm,hvc-id : HVC id required when using hvc transports
>
> Don't the SMC ids get standardized?
>

Unfortunately no. Please don't ask me why ;) as there's no technical reason
to not standardize it. I pushed hard for it, but for this particular case the
SMC/HVC transport is considered to be outside the SCMI protocol standards as
it's transport related.

I completely agree it is total non-sense. I will try to convince them showing
this discussion as reference but can't guarantee anything.

--
Regards,
Sudeep

RE: [PATCH 1/2] dt-bindings: arm: arm,scmi: add smc/hvc transports

[Peng Fan]

> Subject: Re: [PATCH 1/2] dt-bindings: arm: arm,scmi: add smc/hvc transports
> 
> On 2020-02-07 10:47, Sudeep Holla wrote:
> > On Fri, Feb 07, 2020 at 10:08:36AM +0000, Marc Zyngier wrote:
> >> On 2020-02-06 13:01, peng.fan@nxp.com wrote:
> >> > From: Peng Fan <peng.fan@nxp.com>
> >> >
> >> > SCMI could use SMC/HVC as tranports, so add into devicetree binding
> >> > doc.
> >> >
> >> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> >> > ---
> >> >  Documentation/devicetree/bindings/arm/arm,scmi.txt | 4 +++-
> >> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/Documentation/devicetree/bindings/arm/arm,scmi.txt
> >> > b/Documentation/devicetree/bindings/arm/arm,scmi.txt
> >> > index f493d69e6194..03cff8b55a93 100644
> >> > --- a/Documentation/devicetree/bindings/arm/arm,scmi.txt
> >> > +++ b/Documentation/devicetree/bindings/arm/arm,scmi.txt
> >> > @@ -14,7 +14,7 @@ Required properties:
> >> >
> >> >  The scmi node with the following properties shall be under the
> >> > /firmware/ node.
> >> >
> >> > -- compatible : shall be "arm,scmi"
> >> > +- compatible : shall be "arm,scmi" or "arm,scmi-smc"
> >> >  - mboxes: List of phandle and mailbox channel specifiers. It
> >> > should contain
> >> >  	  exactly one or two mailboxes, one for transmitting messages("tx")
> >> >  	  and another optional for receiving the notifications("rx") if
> >> > @@ -25,6 +25,8 @@ The scmi node with the following properties shall
> >> > be under the /firmware/ node.
> >> >  	  protocol identifier for a given sub-node.
> >> >  - #size-cells : should be '0' as 'reg' property doesn't have any size
> >> >  	  associated with it.
> >> > +- arm,smc-id : SMC id required when using smc transports
> >> > +- arm,hvc-id : HVC id required when using hvc transports
> >> >
> >> >  Optional properties:
> >>
> >> Not directly related to DT: Why do we need to distinguish between SMC
> >> and HVC?
> >
> > IIUC you want just one property to get the function ID ? Does that
> > align with what you are saying ? I wanted to ask the same question and
> > I see no need for 2 different properties.
> 
> Exactly. Using SMC or HVC should come from the context, and there is zero
> value in having different different IDs, depending on the conduit.
> 
> We *really* want SMC and HVC to behave the same way. Any attempt to
> make them different should just be NAKed.

ok. Then just like psci node,
Add a "method" property for each protocol, and add "arm,func-id" to
indicate the ID.

How about this?

Thanks,
Peng.



> 
> Thanks,
> 
>          M.
> --
> Jazz is not dead. It just smells funny...

Re: [PATCH 1/2] dt-bindings: arm: arm,scmi: add smc/hvc transports

[Marc Zyngier]

On 2020-02-07 10:55, Peng Fan wrote:
>> Subject: Re: [PATCH 1/2] dt-bindings: arm: arm,scmi: add smc/hvc 
>> transports
>> 
>> On Fri, Feb 07, 2020 at 10:08:36AM +0000, Marc Zyngier wrote:
>> > On 2020-02-06 13:01, peng.fan@nxp.com wrote:
>> > > From: Peng Fan <peng.fan@nxp.com>
>> > >
>> > > SCMI could use SMC/HVC as tranports, so add into devicetree binding
>> > > doc.
>> > >
>> > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
>> > > ---
>> > >  Documentation/devicetree/bindings/arm/arm,scmi.txt | 4 +++-
>> > >  1 file changed, 3 insertions(+), 1 deletion(-)
>> > >
>> > > diff --git a/Documentation/devicetree/bindings/arm/arm,scmi.txt
>> > > b/Documentation/devicetree/bindings/arm/arm,scmi.txt
>> > > index f493d69e6194..03cff8b55a93 100644
>> > > --- a/Documentation/devicetree/bindings/arm/arm,scmi.txt
>> > > +++ b/Documentation/devicetree/bindings/arm/arm,scmi.txt
>> > > @@ -14,7 +14,7 @@ Required properties:
>> > >
>> > >  The scmi node with the following properties shall be under the
>> > > /firmware/ node.
>> > >
>> > > -- compatible : shall be "arm,scmi"
>> > > +- compatible : shall be "arm,scmi" or "arm,scmi-smc"
>> > >  - mboxes: List of phandle and mailbox channel specifiers. It should
>> > > contain
>> > >  	  exactly one or two mailboxes, one for transmitting messages("tx")
>> > >  	  and another optional for receiving the notifications("rx") if @@
>> > > -25,6 +25,8 @@ The scmi node with the following properties shall be
>> > > under the /firmware/ node.
>> > >  	  protocol identifier for a given sub-node.
>> > >  - #size-cells : should be '0' as 'reg' property doesn't have any size
>> > >  	  associated with it.
>> > > +- arm,smc-id : SMC id required when using smc transports
>> > > +- arm,hvc-id : HVC id required when using hvc transports
>> > >
>> > >  Optional properties:
>> >
>> > Not directly related to DT: Why do we need to distinguish between SMC
>> > and HVC?
>> 
>> IIUC you want just one property to get the function ID ? Does that 
>> align with
>> what you are saying ? I wanted to ask the same question and I see no 
>> need for
>> 2 different properties.
> 
> The multiple protocols might use SMC or HVC. Saying
> 
>  Protocol@x {
>     method="smc";
>     arm,func-id=<0x....>
>  };
>  Protocol@y {
>     method="hvc";
>     arm,func-id=<0x....>
>  };
> 
> With my propose:
> 
> Protocol@x {
>     arm,smc-id=<0x....>
>  };
>  Protocol@y {
>     arm,hvc-id=<0x....>
>  };
> 
> No need an extra method property to indicate it is smc or hvc.
> The driver use take arm,smc-id as SMC, arm,hvc-id as HVC.

You're missing the point. I do not want to see different IDs depending 
on
Whether we use HVC and SMC. They *MUST* have the same value, and the 
right
way to enforce this is to disallow any indication of the conduit.

An even better thing would be for ARM to mandate the ID, so that vendors
can be deprived of their special "value add" once and for all.

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH 1/2] dt-bindings: arm: arm,scmi: add smc/hvc transports

[Sudeep Holla]

On Fri, Feb 07, 2020 at 10:55:44AM +0000, Peng Fan wrote:
> > Subject: Re: [PATCH 1/2] dt-bindings: arm: arm,scmi: add smc/hvc transports
> >
> > On Fri, Feb 07, 2020 at 10:08:36AM +0000, Marc Zyngier wrote:
> > > On 2020-02-06 13:01, peng.fan@nxp.com wrote:
> > > > From: Peng Fan <peng.fan@nxp.com>
> > > >
> > > > SCMI could use SMC/HVC as tranports, so add into devicetree binding
> > > > doc.
> > > >
> > > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > > ---
> > > >  Documentation/devicetree/bindings/arm/arm,scmi.txt | 4 +++-
> > > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > > > b/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > > > index f493d69e6194..03cff8b55a93 100644
> > > > --- a/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > > > +++ b/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > > > @@ -14,7 +14,7 @@ Required properties:
> > > >
> > > >  The scmi node with the following properties shall be under the
> > > > /firmware/ node.
> > > >
> > > > -- compatible : shall be "arm,scmi"
> > > > +- compatible : shall be "arm,scmi" or "arm,scmi-smc"
> > > >  - mboxes: List of phandle and mailbox channel specifiers. It should
> > > > contain
> > > >  	  exactly one or two mailboxes, one for transmitting messages("tx")
> > > >  	  and another optional for receiving the notifications("rx") if @@
> > > > -25,6 +25,8 @@ The scmi node with the following properties shall be
> > > > under the /firmware/ node.
> > > >  	  protocol identifier for a given sub-node.
> > > >  - #size-cells : should be '0' as 'reg' property doesn't have any size
> > > >  	  associated with it.
> > > > +- arm,smc-id : SMC id required when using smc transports
> > > > +- arm,hvc-id : HVC id required when using hvc transports
> > > >
> > > >  Optional properties:
> > >
> > > Not directly related to DT: Why do we need to distinguish between SMC
> > > and HVC?
> >
> > IIUC you want just one property to get the function ID ? Does that align with
> > what you are saying ? I wanted to ask the same question and I see no need for
> > 2 different properties.
>
> The multiple protocols might use SMC or HVC. Saying
>
>  Protocol@x {
>     method="smc";
>     arm,func-id=<0x....>
>  };
>  Protocol@y {
>     method="hvc";
>     arm,func-id=<0x....>
>  };
>

Wow, stop there. Please don't do that. You either use SMC or HVC consistently.
Not both at the same time. Any particular reasons for trying such crazy things.

> With my propose:
>
> Protocol@x {
>     arm,smc-id=<0x....>
>  };
>  Protocol@y {
>     arm,hvc-id=<0x....>
>  };
>
> No need an extra method property to indicate it is smc or hvc.
> The driver use take arm,smc-id as SMC, arm,hvc-id as HVC.
>

NACK, just have one function ID, I am not very particular on the name
'smc-id' is just fine for me. But only one function ID for any conduit
used and that is chosen by PSCI/SMCCC.

If you need multiple channels(unique per protocol) then I suggest go for
an channel ID or you can even manage just with shmem associated with it
(I prefer latter but again I am fine either way)

--
Regards,
Sudeep

Re: [PATCH 1/2] dt-bindings: arm: arm,scmi: add smc/hvc transports

[Marc Zyngier]

On 2020-02-07 11:00, Peng Fan wrote:
>> Subject: Re: [PATCH 1/2] dt-bindings: arm: arm,scmi: add smc/hvc 
>> transports
>> 
>> On 2020-02-07 10:47, Sudeep Holla wrote:
>> > On Fri, Feb 07, 2020 at 10:08:36AM +0000, Marc Zyngier wrote:
>> >> On 2020-02-06 13:01, peng.fan@nxp.com wrote:
>> >> > From: Peng Fan <peng.fan@nxp.com>
>> >> >
>> >> > SCMI could use SMC/HVC as tranports, so add into devicetree binding
>> >> > doc.
>> >> >
>> >> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
>> >> > ---
>> >> >  Documentation/devicetree/bindings/arm/arm,scmi.txt | 4 +++-
>> >> >  1 file changed, 3 insertions(+), 1 deletion(-)
>> >> >
>> >> > diff --git a/Documentation/devicetree/bindings/arm/arm,scmi.txt
>> >> > b/Documentation/devicetree/bindings/arm/arm,scmi.txt
>> >> > index f493d69e6194..03cff8b55a93 100644
>> >> > --- a/Documentation/devicetree/bindings/arm/arm,scmi.txt
>> >> > +++ b/Documentation/devicetree/bindings/arm/arm,scmi.txt
>> >> > @@ -14,7 +14,7 @@ Required properties:
>> >> >
>> >> >  The scmi node with the following properties shall be under the
>> >> > /firmware/ node.
>> >> >
>> >> > -- compatible : shall be "arm,scmi"
>> >> > +- compatible : shall be "arm,scmi" or "arm,scmi-smc"
>> >> >  - mboxes: List of phandle and mailbox channel specifiers. It
>> >> > should contain
>> >> >  	  exactly one or two mailboxes, one for transmitting messages("tx")
>> >> >  	  and another optional for receiving the notifications("rx") if
>> >> > @@ -25,6 +25,8 @@ The scmi node with the following properties shall
>> >> > be under the /firmware/ node.
>> >> >  	  protocol identifier for a given sub-node.
>> >> >  - #size-cells : should be '0' as 'reg' property doesn't have any size
>> >> >  	  associated with it.
>> >> > +- arm,smc-id : SMC id required when using smc transports
>> >> > +- arm,hvc-id : HVC id required when using hvc transports
>> >> >
>> >> >  Optional properties:
>> >>
>> >> Not directly related to DT: Why do we need to distinguish between SMC
>> >> and HVC?
>> >
>> > IIUC you want just one property to get the function ID ? Does that
>> > align with what you are saying ? I wanted to ask the same question and
>> > I see no need for 2 different properties.
>> 
>> Exactly. Using SMC or HVC should come from the context, and there is 
>> zero
>> value in having different different IDs, depending on the conduit.
>> 
>> We *really* want SMC and HVC to behave the same way. Any attempt to
>> make them different should just be NAKed.
> 
> ok. Then just like psci node,
> Add a "method" property for each protocol, and add "arm,func-id" to
> indicate the ID.
> 
> How about this?

Or rather just a function ID, full stop. the conduit *MUST* be inherited
from the PSCI context.

         M.
-- 
Jazz is not dead. It just smells funny...

RE: [PATCH 1/2] dt-bindings: arm: arm,scmi: add smc/hvc transports

[Peng Fan]

> Subject: Re: [PATCH 1/2] dt-bindings: arm: arm,scmi: add smc/hvc transports
> 
> On Fri, Feb 07, 2020 at 10:55:44AM +0000, Peng Fan wrote:
> > > Subject: Re: [PATCH 1/2] dt-bindings: arm: arm,scmi: add smc/hvc
> > > transports
> > >
> > > On Fri, Feb 07, 2020 at 10:08:36AM +0000, Marc Zyngier wrote:
> > > > On 2020-02-06 13:01, peng.fan@nxp.com wrote:
> > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > >
> > > > > SCMI could use SMC/HVC as tranports, so add into devicetree
> > > > > binding doc.
> > > > >
> > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > > > ---
> > > > >  Documentation/devicetree/bindings/arm/arm,scmi.txt | 4 +++-
> > > > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > > > > b/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > > > > index f493d69e6194..03cff8b55a93 100644
> > > > > --- a/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > > > > +++ b/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > > > > @@ -14,7 +14,7 @@ Required properties:
> > > > >
> > > > >  The scmi node with the following properties shall be under the
> > > > > /firmware/ node.
> > > > >
> > > > > -- compatible : shall be "arm,scmi"
> > > > > +- compatible : shall be "arm,scmi" or "arm,scmi-smc"
> > > > >  - mboxes: List of phandle and mailbox channel specifiers. It
> > > > > should contain
> > > > >  	  exactly one or two mailboxes, one for transmitting messages("tx")
> > > > >  	  and another optional for receiving the notifications("rx")
> > > > > if @@
> > > > > -25,6 +25,8 @@ The scmi node with the following properties shall
> > > > > be under the /firmware/ node.
> > > > >  	  protocol identifier for a given sub-node.
> > > > >  - #size-cells : should be '0' as 'reg' property doesn't have any size
> > > > >  	  associated with it.
> > > > > +- arm,smc-id : SMC id required when using smc transports
> > > > > +- arm,hvc-id : HVC id required when using hvc transports
> > > > >
> > > > >  Optional properties:
> > > >
> > > > Not directly related to DT: Why do we need to distinguish between
> > > > SMC and HVC?
> > >
> > > IIUC you want just one property to get the function ID ? Does that
> > > align with what you are saying ? I wanted to ask the same question
> > > and I see no need for
> > > 2 different properties.
> >
> > The multiple protocols might use SMC or HVC. Saying
> >
> >  Protocol@x {
> >     method="smc";
> >     arm,func-id=<0x....>
> >  };
> >  Protocol@y {
> >     method="hvc";
> >     arm,func-id=<0x....>
> >  };
> >
> 
> Wow, stop there. Please don't do that. You either use SMC or HVC
> consistently.
> Not both at the same time. Any particular reasons for trying such crazy things.
> 
> > With my propose:
> >
> > Protocol@x {
> >     arm,smc-id=<0x....>
> >  };
> >  Protocol@y {
> >     arm,hvc-id=<0x....>
> >  };
> >
> > No need an extra method property to indicate it is smc or hvc.
> > The driver use take arm,smc-id as SMC, arm,hvc-id as HVC.
> >
> 
> NACK, just have one function ID, I am not very particular on the name 'smc-id'
> is just fine for me. But only one function ID for any conduit used and that is
> chosen by PSCI/SMCCC.
> 
> If you need multiple channels(unique per protocol) then I suggest go for an
> channel ID or you can even manage just with shmem associated with it (I
> prefer latter but again I am fine either way)

Ok. Just follow Marc suggested
Parse the conduit from PSCI context. Then only add 'smc-id' property in scmi
node, and take protocol reg as arg1. Is this ok for you?

Thanks,
Peng.

> 
> --
> Regards,
> Sudeep

Re: [PATCH 1/2] dt-bindings: arm: arm,scmi: add smc/hvc transports

[Sudeep Holla]

On Fri, Feb 07, 2020 at 11:09:48AM +0000, Marc Zyngier wrote:
> On 2020-02-07 11:00, Peng Fan wrote:
> > > Subject: Re: [PATCH 1/2] dt-bindings: arm: arm,scmi: add smc/hvc
> > > transports
> > > 
> > > On 2020-02-07 10:47, Sudeep Holla wrote:
> > > > On Fri, Feb 07, 2020 at 10:08:36AM +0000, Marc Zyngier wrote:
> > > >> On 2020-02-06 13:01, peng.fan@nxp.com wrote:
> > > >> > From: Peng Fan <peng.fan@nxp.com>
> > > >> >
> > > >> > SCMI could use SMC/HVC as tranports, so add into devicetree binding
> > > >> > doc.
> > > >> >
> > > >> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > >> > ---
> > > >> >  Documentation/devicetree/bindings/arm/arm,scmi.txt | 4 +++-
> > > >> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > >> >
> > > >> > diff --git a/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > > >> > b/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > > >> > index f493d69e6194..03cff8b55a93 100644
> > > >> > --- a/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > > >> > +++ b/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > > >> > @@ -14,7 +14,7 @@ Required properties:
> > > >> >
> > > >> >  The scmi node with the following properties shall be under the
> > > >> > /firmware/ node.
> > > >> >
> > > >> > -- compatible : shall be "arm,scmi"
> > > >> > +- compatible : shall be "arm,scmi" or "arm,scmi-smc"
> > > >> >  - mboxes: List of phandle and mailbox channel specifiers. It
> > > >> > should contain
> > > >> >  	  exactly one or two mailboxes, one for transmitting messages("tx")
> > > >> >  	  and another optional for receiving the notifications("rx") if
> > > >> > @@ -25,6 +25,8 @@ The scmi node with the following properties shall
> > > >> > be under the /firmware/ node.
> > > >> >  	  protocol identifier for a given sub-node.
> > > >> >  - #size-cells : should be '0' as 'reg' property doesn't have any size
> > > >> >  	  associated with it.
> > > >> > +- arm,smc-id : SMC id required when using smc transports
> > > >> > +- arm,hvc-id : HVC id required when using hvc transports
> > > >> >
> > > >> >  Optional properties:
> > > >>
> > > >> Not directly related to DT: Why do we need to distinguish between SMC
> > > >> and HVC?
> > > >
> > > > IIUC you want just one property to get the function ID ? Does that
> > > > align with what you are saying ? I wanted to ask the same question and
> > > > I see no need for 2 different properties.
> > > 
> > > Exactly. Using SMC or HVC should come from the context, and there is
> > > zero
> > > value in having different different IDs, depending on the conduit.
> > > 
> > > We *really* want SMC and HVC to behave the same way. Any attempt to
> > > make them different should just be NAKed.
> > 
> > ok. Then just like psci node,
> > Add a "method" property for each protocol, and add "arm,func-id" to
> > indicate the ID.
> > 
> > How about this?
> 
> Or rather just a function ID, full stop. the conduit *MUST* be inherited
> from the PSCI context.

Absolutely, this is what I was expecting.

Peng,

You have already introduced a compatible for smc/hvc transport
instead of default mailbox, why do you need anything more ? Just
use SMC or HVC conduit from PSCI/SMCCC. I don't think you need anything
more than the function ID.

--
Regards,
Sudeep

RE: [PATCH 1/2] dt-bindings: arm: arm,scmi: add smc/hvc transports

[Peng Fan]

> Subject: Re: [PATCH 1/2] dt-bindings: arm: arm,scmi: add smc/hvc transports
> 
> On Fri, Feb 07, 2020 at 11:09:48AM +0000, Marc Zyngier wrote:
> > On 2020-02-07 11:00, Peng Fan wrote:
> > > > Subject: Re: [PATCH 1/2] dt-bindings: arm: arm,scmi: add smc/hvc
> > > > transports
> > > >
> > > > On 2020-02-07 10:47, Sudeep Holla wrote:
> > > > > On Fri, Feb 07, 2020 at 10:08:36AM +0000, Marc Zyngier wrote:
> > > > >> On 2020-02-06 13:01, peng.fan@nxp.com wrote:
> > > > >> > From: Peng Fan <peng.fan@nxp.com>
> > > > >> >
> > > > >> > SCMI could use SMC/HVC as tranports, so add into devicetree
> > > > >> > binding doc.
> > > > >> >
> > > > >> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > > >> > ---
> > > > >> >  Documentation/devicetree/bindings/arm/arm,scmi.txt | 4 +++-
> > > > >> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > > >> >
> > > > >> > diff --git
> > > > >> > a/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > > > >> > b/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > > > >> > index f493d69e6194..03cff8b55a93 100644
> > > > >> > --- a/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > > > >> > +++ b/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > > > >> > @@ -14,7 +14,7 @@ Required properties:
> > > > >> >
> > > > >> >  The scmi node with the following properties shall be under
> > > > >> > the /firmware/ node.
> > > > >> >
> > > > >> > -- compatible : shall be "arm,scmi"
> > > > >> > +- compatible : shall be "arm,scmi" or "arm,scmi-smc"
> > > > >> >  - mboxes: List of phandle and mailbox channel specifiers. It
> > > > >> > should contain
> > > > >> >  	  exactly one or two mailboxes, one for transmitting
> messages("tx")
> > > > >> >  	  and another optional for receiving the
> > > > >> > notifications("rx") if @@ -25,6 +25,8 @@ The scmi node with
> > > > >> > the following properties shall be under the /firmware/ node.
> > > > >> >  	  protocol identifier for a given sub-node.
> > > > >> >  - #size-cells : should be '0' as 'reg' property doesn't have any size
> > > > >> >  	  associated with it.
> > > > >> > +- arm,smc-id : SMC id required when using smc transports
> > > > >> > +- arm,hvc-id : HVC id required when using hvc transports
> > > > >> >
> > > > >> >  Optional properties:
> > > > >>
> > > > >> Not directly related to DT: Why do we need to distinguish
> > > > >> between SMC and HVC?
> > > > >
> > > > > IIUC you want just one property to get the function ID ? Does
> > > > > that align with what you are saying ? I wanted to ask the same
> > > > > question and I see no need for 2 different properties.
> > > >
> > > > Exactly. Using SMC or HVC should come from the context, and there
> > > > is zero value in having different different IDs, depending on the
> > > > conduit.
> > > >
> > > > We *really* want SMC and HVC to behave the same way. Any attempt
> > > > to make them different should just be NAKed.
> > >
> > > ok. Then just like psci node,
> > > Add a "method" property for each protocol, and add "arm,func-id" to
> > > indicate the ID.
> > >
> > > How about this?
> >
> > Or rather just a function ID, full stop. the conduit *MUST* be
> > inherited from the PSCI context.
> 
> Absolutely, this is what I was expecting.
> 
> Peng,
> 
> You have already introduced a compatible for smc/hvc transport instead of
> default mailbox, why do you need anything more ? 

No.

Just use SMC or HVC
> conduit from PSCI/SMCCC. I don't think you need anything more than the
> function ID.

Yes, only function ID for now. If function ID could not be standardized in short
term, I could first mark smc-id optional in dt bindings, then smc transports
driver will first parse smc-id, abort if not exist. When ARM has standarlized
ID, we could switch to use that ID if smc-id not exist.

Thanks,
Peng.

> 
> --
> Regards,
> Sudeep

RE: [PATCH 1/2] dt-bindings: arm: arm,scmi: add smc/hvc transports

[Peng Fan]

> Subject: [PATCH 1/2] dt-bindings: arm: arm,scmi: add smc/hvc transports
> 
> From: Peng Fan <peng.fan@nxp.com>
> 
> SCMI could use SMC/HVC as tranports, so add into devicetree binding doc.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  Documentation/devicetree/bindings/arm/arm,scmi.txt | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/arm/arm,scmi.txt
> b/Documentation/devicetree/bindings/arm/arm,scmi.txt
> index f493d69e6194..03cff8b55a93 100644
> --- a/Documentation/devicetree/bindings/arm/arm,scmi.txt
> +++ b/Documentation/devicetree/bindings/arm/arm,scmi.txt
> @@ -14,7 +14,7 @@ Required properties:
> 
>  The scmi node with the following properties shall be under the /firmware/
> node.
> 
> -- compatible : shall be "arm,scmi"
> +- compatible : shall be "arm,scmi" or "arm,scmi-smc"

One question here, are you fine with compatible "arm,scmi-smc" or
add a new property 'transport' such as:
transport = "mailbox"; for mailbox
transport = "smc"; for smc and hvc.

Thanks,
Peng.

>  - mboxes: List of phandle and mailbox channel specifiers. It should contain
>  	  exactly one or two mailboxes, one for transmitting messages("tx")
>  	  and another optional for receiving the notifications("rx") if @@ -25,6
> +25,8 @@ The scmi node with the following properties shall be under the
> /firmware/ node.
>  	  protocol identifier for a given sub-node.
>  - #size-cells : should be '0' as 'reg' property doesn't have any size
>  	  associated with it.
> +- arm,smc-id : SMC id required when using smc transports
> +- arm,hvc-id : HVC id required when using hvc transports
> 
>  Optional properties:
> 
> --
> 2.16.4

Re: [PATCH 1/2] dt-bindings: arm: arm,scmi: add smc/hvc transports

[Sudeep Holla]

On Mon, Feb 10, 2020 at 12:59:43AM +0000, Peng Fan wrote:
> > Subject: [PATCH 1/2] dt-bindings: arm: arm,scmi: add smc/hvc transports
> >
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > SCMI could use SMC/HVC as tranports, so add into devicetree binding doc.
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >  Documentation/devicetree/bindings/arm/arm,scmi.txt | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > b/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > index f493d69e6194..03cff8b55a93 100644
> > --- a/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > +++ b/Documentation/devicetree/bindings/arm/arm,scmi.txt
> > @@ -14,7 +14,7 @@ Required properties:
> >
> >  The scmi node with the following properties shall be under the /firmware/
> > node.
> >
> > -- compatible : shall be "arm,scmi"
> > +- compatible : shall be "arm,scmi" or "arm,scmi-smc"
>
> One question here, are you fine with compatible "arm,scmi-smc" or
> add a new property 'transport' such as:

Not required.

> transport = "mailbox"; for mailbox
> transport = "smc"; for smc and hvc.
>
Each transport will have it's own specific property like mailboxes for
mailbox, smc/hvc function IDs for SMC/HVC. That should be sufficient.

--
Regards,
Sudeep