* [PATCH V2 0/2] firmware: arm_scmi: add smc/hvc transports support @ 2020-02-13 3:58 peng.fan 2020-02-13 3:58 ` [PATCH V2 1/2] dt-bindings: arm: arm,scmi: add smc/hvc transports peng.fan 2020-02-13 3:58 ` [PATCH V2 2/2] firmware: arm_scmi: " peng.fan 0 siblings, 2 replies; 7+ messages in thread From: peng.fan @ 2020-02-13 3:58 UTC (permalink / raw) To: sudeep.holla, robh+dt, mark.rutland Cc: viresh.kumar, f.fainelli, linux-imx, linux-arm-kernel, linux-kernel, devicetree, andre.przywara, Peng Fan From: Peng Fan <peng.fan@nxp.com> V2: patch 1/2: only add smc-id property patch 2/2: Parse smc/hvc from psci node Use prot_id as 2nd arg when issue smc/hvc Differentiate tranports using mboxes or smc-id property 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. All protocol share same smc/hvc id, the protocol id will be take as 2nd arg when issue smc/hvc. 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 | 1 + drivers/firmware/arm_scmi/Makefile | 2 +- drivers/firmware/arm_scmi/common.h | 4 +- drivers/firmware/arm_scmi/driver.c | 11 +- drivers/firmware/arm_scmi/mailbox.c | 2 +- drivers/firmware/arm_scmi/smc.c | 222 +++++++++++++++++++++ 6 files changed, 235 insertions(+), 7 deletions(-) create mode 100644 drivers/firmware/arm_scmi/smc.c -- 2.16.4 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH V2 1/2] dt-bindings: arm: arm,scmi: add smc/hvc transports 2020-02-13 3:58 [PATCH V2 0/2] firmware: arm_scmi: add smc/hvc transports support peng.fan @ 2020-02-13 3:58 ` peng.fan 2020-02-13 10:54 ` Sudeep Holla 2020-02-13 3:58 ` [PATCH V2 2/2] firmware: arm_scmi: " peng.fan 1 sibling, 1 reply; 7+ messages in thread From: peng.fan @ 2020-02-13 3:58 UTC (permalink / raw) To: sudeep.holla, robh+dt, mark.rutland Cc: viresh.kumar, f.fainelli, linux-imx, linux-arm-kernel, linux-kernel, devicetree, andre.przywara, Peng Fan From: Peng Fan <peng.fan@nxp.com> SCMI could use SMC/HVC as tranports. Since there is no standardized id, we need to use vendor specific id. So add into devicetree binding doc. Signed-off-by: Peng Fan <peng.fan@nxp.com> --- Documentation/devicetree/bindings/arm/arm,scmi.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/arm/arm,scmi.txt b/Documentation/devicetree/bindings/arm/arm,scmi.txt index f493d69e6194..dacc62dc248b 100644 --- a/Documentation/devicetree/bindings/arm/arm,scmi.txt +++ b/Documentation/devicetree/bindings/arm/arm,scmi.txt @@ -25,6 +25,7 @@ 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. +- smc-id : SMC id required when using smc or hvc transports Optional properties: -- 2.16.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH V2 1/2] dt-bindings: arm: arm,scmi: add smc/hvc transports 2020-02-13 3:58 ` [PATCH V2 1/2] dt-bindings: arm: arm,scmi: add smc/hvc transports peng.fan @ 2020-02-13 10:54 ` Sudeep Holla 2020-02-14 0:59 ` Peng Fan 0 siblings, 1 reply; 7+ messages in thread From: Sudeep Holla @ 2020-02-13 10:54 UTC (permalink / raw) To: peng.fan Cc: robh+dt, mark.rutland, viresh.kumar, f.fainelli, linux-imx, linux-arm-kernel, linux-kernel, devicetree, andre.przywara, Sudeep Holla On Thu, Feb 13, 2020 at 11:58:49AM +0800, peng.fan@nxp.com wrote: > From: Peng Fan <peng.fan@nxp.com> > > SCMI could use SMC/HVC as tranports. Since there is no > standardized id, we need to use vendor specific id. So > add into devicetree binding doc. > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > --- > Documentation/devicetree/bindings/arm/arm,scmi.txt | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/Documentation/devicetree/bindings/arm/arm,scmi.txt b/Documentation/devicetree/bindings/arm/arm,scmi.txt > index f493d69e6194..dacc62dc248b 100644 > --- a/Documentation/devicetree/bindings/arm/arm,scmi.txt > +++ b/Documentation/devicetree/bindings/arm/arm,scmi.txt > @@ -25,6 +25,7 @@ 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. > +- smc-id : SMC id required when using smc or hvc transports IIUC, "arm,smc-id" is preferred more. Why did you drop "arm,scmi-smc" ? -- Regards, Sudeep ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH V2 1/2] dt-bindings: arm: arm,scmi: add smc/hvc transports 2020-02-13 10:54 ` Sudeep Holla @ 2020-02-14 0:59 ` Peng Fan 0 siblings, 0 replies; 7+ messages in thread From: Peng Fan @ 2020-02-14 0:59 UTC (permalink / raw) To: Sudeep Holla Cc: robh+dt, mark.rutland, viresh.kumar, f.fainelli, dl-linux-imx, linux-arm-kernel, linux-kernel, devicetree, andre.przywara Hi Sudeep, > Subject: Re: [PATCH V2 1/2] dt-bindings: arm: arm,scmi: add smc/hvc > transports > > On Thu, Feb 13, 2020 at 11:58:49AM +0800, peng.fan@nxp.com wrote: > > From: Peng Fan <peng.fan@nxp.com> > > > > SCMI could use SMC/HVC as tranports. Since there is no standardized > > id, we need to use vendor specific id. So add into devicetree binding > > doc. > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > --- > > Documentation/devicetree/bindings/arm/arm,scmi.txt | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/Documentation/devicetree/bindings/arm/arm,scmi.txt > > b/Documentation/devicetree/bindings/arm/arm,scmi.txt > > index f493d69e6194..dacc62dc248b 100644 > > --- a/Documentation/devicetree/bindings/arm/arm,scmi.txt > > +++ b/Documentation/devicetree/bindings/arm/arm,scmi.txt > > @@ -25,6 +25,7 @@ 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. > > +- smc-id : SMC id required when using smc or hvc transports > > IIUC, "arm,smc-id" is preferred more. ok. Fix in v3. > > Why did you drop "arm,scmi-smc" ? Per our discuss in v1 patchset, mailbox/smc-id could be used to differentiate mailbox and smc transports. https://lkml.org/lkml/2020/2/11/226 So I still use "arm,scmi" for smc tranports. Thanks, Peng. > > -- > Regards, > Sudeep ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH V2 2/2] firmware: arm_scmi: add smc/hvc transports 2020-02-13 3:58 [PATCH V2 0/2] firmware: arm_scmi: add smc/hvc transports support peng.fan 2020-02-13 3:58 ` [PATCH V2 1/2] dt-bindings: arm: arm,scmi: add smc/hvc transports peng.fan @ 2020-02-13 3:58 ` peng.fan 2020-02-13 11:04 ` Sudeep Holla 1 sibling, 1 reply; 7+ messages in thread From: peng.fan @ 2020-02-13 3:58 UTC (permalink / raw) To: sudeep.holla, robh+dt, mark.rutland Cc: viresh.kumar, f.fainelli, linux-imx, linux-arm-kernel, linux-kernel, devicetree, andre.przywara, Peng Fan From: Peng Fan <peng.fan@nxp.com> Add SCMI smc/hvc transports support. Take smc-id as the 2nd arg, and protocol id as the 2nd arg when invokding SMC/HVC. Since we need protocol id, so add this parrameter to chan_setup, then smc transport driver could directly use it. 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 | 4 +- drivers/firmware/arm_scmi/driver.c | 11 +- drivers/firmware/arm_scmi/mailbox.c | 2 +- drivers/firmware/arm_scmi/smc.c | 222 ++++++++++++++++++++++++++++++++++++ 5 files changed, 234 insertions(+), 7 deletions(-) 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 fd091a4ccbff..b8cd33be8272 100644 --- a/drivers/firmware/arm_scmi/common.h +++ b/drivers/firmware/arm_scmi/common.h @@ -182,7 +182,8 @@ struct scmi_chan_info { */ struct scmi_transport_ops { bool (*chan_available)(struct device *dev, int idx); - int (*chan_setup)(struct scmi_chan_info *cinfo, struct device *dev, bool tx); + int (*chan_setup)(struct scmi_chan_info *cinfo, struct device *dev, + int prot_id, bool tx); int (*chan_free)(int id, void *p, void *data); int (*send_message)(struct scmi_chan_info *cinfo, struct scmi_xfer *xfer); void (*mark_txdone)(struct scmi_chan_info *cinfo, int ret); @@ -207,6 +208,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..65c56328e6d8 100644 --- a/drivers/firmware/arm_scmi/driver.c +++ b/drivers/firmware/arm_scmi/driver.c @@ -595,7 +595,7 @@ static int scmi_chan_setup(struct scmi_info *info, struct device *dev, cinfo->dev = dev; - ret = info->desc->ops->chan_setup(cinfo, info->dev, tx); + ret = info->desc->ops->chan_setup(cinfo, info->dev, prot_id, tx); if (ret) return ret; @@ -687,8 +687,11 @@ static int scmi_probe(struct platform_device *pdev) struct device *dev = &pdev->dev; struct device_node *child, *np = dev->of_node; - desc = of_device_get_match_data(dev); - if (!desc) + if (of_get_property(np, "mboxes", NULL)) + desc = &scmi_mailbox_desc; + else if (of_get_property(np, "smc-id", NULL)) + desc = &scmi_smc_desc; + else return -EINVAL; info = devm_kzalloc(dev, sizeof(*info), GFP_KERNEL); @@ -826,7 +829,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", }, { /* Sentinel */ }, }; diff --git a/drivers/firmware/arm_scmi/mailbox.c b/drivers/firmware/arm_scmi/mailbox.c index 73077bbc4ad9..b06d5d30fe90 100644 --- a/drivers/firmware/arm_scmi/mailbox.c +++ b/drivers/firmware/arm_scmi/mailbox.c @@ -53,7 +53,7 @@ static bool mailbox_chan_available(struct device *dev, int idx) } static int mailbox_chan_setup(struct scmi_chan_info *cinfo, struct device *dev, - bool tx) + int prot_id, bool tx) { const char *desc = tx ? "Tx" : "Rx"; struct device *cdev = cinfo->dev; diff --git a/drivers/firmware/arm_scmi/smc.c b/drivers/firmware/arm_scmi/smc.c new file mode 100644 index 000000000000..67bbf33efb1d --- /dev/null +++ b/drivers/firmware/arm_scmi/smc.c @@ -0,0 +1,222 @@ +// 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 + * @prot_id: The protocol id + */ + +struct scmi_smc { + struct scmi_chan_info *cinfo; + struct scmi_shared_mem __iomem *shmem; + u32 func_id; + int prot_id; +}; + +typedef unsigned long (scmi_smc_fn)(unsigned long, unsigned long, + unsigned long, unsigned long, + unsigned long, unsigned long, + unsigned long, unsigned long); +static scmi_smc_fn *invoke_scmi_smc_fn; + +#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 unsigned long +__invoke_scmi_fn_hvc(unsigned long function_id, unsigned long arg0, + unsigned long arg1, unsigned long arg2, + unsigned long arg3, unsigned long arg4, + unsigned long arg5, unsigned long arg6) +{ + struct arm_smccc_res res; + + arm_smccc_hvc(function_id, arg0, arg1, arg2, arg3, arg4, arg5, + arg6, &res); + + return res.a0; +} + +static unsigned long +__invoke_scmi_fn_smc(unsigned long function_id, unsigned long arg0, + unsigned long arg1, unsigned long arg2, + unsigned long arg3, unsigned long arg4, + unsigned long arg5, unsigned long arg6) +{ + struct arm_smccc_res res; + + arm_smccc_smc(function_id, arg0, arg1, arg2, arg3, arg4, arg5, + arg6, &res); + + return res.a0; +} + +static int scmi_smc_conduit_method(struct device_node *np) +{ + const char *method; + + if (invoke_scmi_smc_fn) + return 0; + + if (of_property_read_string(np, "method", &method)) + return -ENXIO; + + if (!strcmp("hvc", method)) { + invoke_scmi_smc_fn = __invoke_scmi_fn_hvc; + } else if (!strcmp("smc", method)) { + invoke_scmi_smc_fn = __invoke_scmi_fn_smc; + } else { + pr_warn("invalid \"method\" property: %s\n", method); + return -EINVAL; + } + + return 0; +} + +static int smc_chan_setup(struct scmi_chan_info *cinfo, struct device *dev, + int prot_id, bool tx) +{ + struct device *cdev = cinfo->dev; + struct scmi_smc *scmi_info; + struct device_node *np; + 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; + + np = of_parse_phandle(cdev->of_node, "shmem", 0); + if (!np) + np = of_parse_phandle(dev->of_node, "shmem", 0); + ret = of_address_to_resource(np, 0, &res); + of_node_put(np); + 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(dev->of_node, "smc-id", &func_id); + if (ret < 0) + return ret; + + np = of_find_node_by_path("/psci"); + if (!np) { + dev_err(dev, "Not able to find /psci node\n"); + return -ENODEV; + } + + ret = scmi_smc_conduit_method(np); + if (ret) + return ret; + + of_node_put(np); + + scmi_info->func_id = func_id; + scmi_info->cinfo = cinfo; + scmi_info->prot_id = prot_id; + cinfo->transport_info = scmi_info; + + 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; + int ret; + + shmem_tx_prepare(scmi_info->shmem, xfer); + + ret = invoke_scmi_smc_fn(scmi_info->func_id, scmi_info->prot_id, + 0, 0, 0, 0, 0, 0); + 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH V2 2/2] firmware: arm_scmi: add smc/hvc transports 2020-02-13 3:58 ` [PATCH V2 2/2] firmware: arm_scmi: " peng.fan @ 2020-02-13 11:04 ` Sudeep Holla 2020-02-14 1:04 ` Peng Fan 0 siblings, 1 reply; 7+ messages in thread From: Sudeep Holla @ 2020-02-13 11:04 UTC (permalink / raw) To: peng.fan Cc: robh+dt, mark.rutland, viresh.kumar, f.fainelli, linux-imx, linux-arm-kernel, linux-kernel, devicetree, andre.przywara, Sudeep Holla On Thu, Feb 13, 2020 at 11:58:50AM +0800, peng.fan@nxp.com wrote: > From: Peng Fan <peng.fan@nxp.com> > > Add SCMI smc/hvc transports support. > > Take smc-id as the 2nd arg, and protocol id as the 2nd arg when > invokding SMC/HVC. Since we need protocol id, so add this parrameter > to chan_setup, then smc transport driver could directly use it. > 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 | 4 +- > drivers/firmware/arm_scmi/driver.c | 11 +- > drivers/firmware/arm_scmi/mailbox.c | 2 +- > drivers/firmware/arm_scmi/smc.c | 222 ++++++++++++++++++++++++++++++++++++ > 5 files changed, 234 insertions(+), 7 deletions(-) > create mode 100644 drivers/firmware/arm_scmi/smc.c [...] > > diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c > index dbec767222e9..65c56328e6d8 100644 > --- a/drivers/firmware/arm_scmi/driver.c > +++ b/drivers/firmware/arm_scmi/driver.c > @@ -595,7 +595,7 @@ static int scmi_chan_setup(struct scmi_info *info, struct device *dev, > > cinfo->dev = dev; > > - ret = info->desc->ops->chan_setup(cinfo, info->dev, tx); > + ret = info->desc->ops->chan_setup(cinfo, info->dev, prot_id, tx); Why do you need this ? > if (ret) > return ret; > > @@ -826,7 +829,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", }, Don't do this, get "arm,scmi-smc" > { /* Sentinel */ }, > }; > [...] > +static unsigned long > +__invoke_scmi_fn_hvc(unsigned long function_id, unsigned long arg0, > + unsigned long arg1, unsigned long arg2, > + unsigned long arg3, unsigned long arg4, > + unsigned long arg5, unsigned long arg6) > +{ > + struct arm_smccc_res res; > + > + arm_smccc_hvc(function_id, arg0, arg1, arg2, arg3, arg4, arg5, > + arg6, &res); > + > + return res.a0; > +} > + > +static unsigned long > +__invoke_scmi_fn_smc(unsigned long function_id, unsigned long arg0, > + unsigned long arg1, unsigned long arg2, > + unsigned long arg3, unsigned long arg4, > + unsigned long arg5, unsigned long arg6) > +{ > + struct arm_smccc_res res; > + > + arm_smccc_smc(function_id, arg0, arg1, arg2, arg3, arg4, arg5, > + arg6, &res); > + > + return res.a0; > +} > + > +static int scmi_smc_conduit_method(struct device_node *np) > +{ > + const char *method; > + > + if (invoke_scmi_smc_fn) > + return 0; > + > + if (of_property_read_string(np, "method", &method)) > + return -ENXIO; > + > + if (!strcmp("hvc", method)) { > + invoke_scmi_smc_fn = __invoke_scmi_fn_hvc; > + } else if (!strcmp("smc", method)) { > + invoke_scmi_smc_fn = __invoke_scmi_fn_smc; > + } else { > + pr_warn("invalid \"method\" property: %s\n", method); > + return -EINVAL; > + } > + > + return 0; > +} > + You don't the above functions [...] > + > + np = of_find_node_by_path("/psci"); > + if (!np) { > + dev_err(dev, "Not able to find /psci node\n"); > + return -ENODEV; > + } No need for this as mentioned below. > + > + ret = scmi_smc_conduit_method(np); Just use arm_smccc_1_1_get_conduit if you want to get the conduit which I don't think you need. You can just use arm_smccc_1_1_invoke() directly. -- Regards, Sudeep ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH V2 2/2] firmware: arm_scmi: add smc/hvc transports 2020-02-13 11:04 ` Sudeep Holla @ 2020-02-14 1:04 ` Peng Fan 0 siblings, 0 replies; 7+ messages in thread From: Peng Fan @ 2020-02-14 1:04 UTC (permalink / raw) To: Sudeep Holla Cc: robh+dt, mark.rutland, viresh.kumar, f.fainelli, dl-linux-imx, linux-arm-kernel, linux-kernel, devicetree, andre.przywara Hi Sudeep, > Subject: Re: [PATCH V2 2/2] firmware: arm_scmi: add smc/hvc transports > > On Thu, Feb 13, 2020 at 11:58:50AM +0800, peng.fan@nxp.com wrote: > > From: Peng Fan <peng.fan@nxp.com> > > > > Add SCMI smc/hvc transports support. > > > > Take smc-id as the 2nd arg, and protocol id as the 2nd arg when > > invokding SMC/HVC. Since we need protocol id, so add this parrameter > > to chan_setup, then smc transport driver could directly use it. > > 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 | 4 +- > > drivers/firmware/arm_scmi/driver.c | 11 +- > > drivers/firmware/arm_scmi/mailbox.c | 2 +- > > drivers/firmware/arm_scmi/smc.c | 222 > ++++++++++++++++++++++++++++++++++++ > > 5 files changed, 234 insertions(+), 7 deletions(-) create mode > > 100644 drivers/firmware/arm_scmi/smc.c > > [...] > > > > > diff --git a/drivers/firmware/arm_scmi/driver.c > > b/drivers/firmware/arm_scmi/driver.c > > index dbec767222e9..65c56328e6d8 100644 > > --- a/drivers/firmware/arm_scmi/driver.c > > +++ b/drivers/firmware/arm_scmi/driver.c > > @@ -595,7 +595,7 @@ static int scmi_chan_setup(struct scmi_info *info, > > struct device *dev, > > > > cinfo->dev = dev; > > > > - ret = info->desc->ops->chan_setup(cinfo, info->dev, tx); > > + ret = info->desc->ops->chan_setup(cinfo, info->dev, prot_id, tx); > > Why do you need this ? For smc tranports, all protocols share same smd-id, but if protocols not share the same shmem, we need let firmare know which protocol issues the smc call. So I take prot_id as an arguments of smc call. > > > if (ret) > > return ret; > > > > @@ -826,7 +829,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", }, > > Don't do this, get "arm,scmi-smc" You mean code as below? /* 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 */ }, }; But since we could use mboxes and smc-id to know the tranports type, do we really need arm,scmi-smc? > > > { /* Sentinel */ }, > > }; > > > [...] > > > > +static unsigned long > > +__invoke_scmi_fn_hvc(unsigned long function_id, unsigned long arg0, > > + unsigned long arg1, unsigned long arg2, > > + unsigned long arg3, unsigned long arg4, > > + unsigned long arg5, unsigned long arg6) { > > + struct arm_smccc_res res; > > + > > + arm_smccc_hvc(function_id, arg0, arg1, arg2, arg3, arg4, arg5, > > + arg6, &res); > > + > > + return res.a0; > > +} > > + > > +static unsigned long > > +__invoke_scmi_fn_smc(unsigned long function_id, unsigned long arg0, > > + unsigned long arg1, unsigned long arg2, > > + unsigned long arg3, unsigned long arg4, > > + unsigned long arg5, unsigned long arg6) { > > + struct arm_smccc_res res; > > + > > + arm_smccc_smc(function_id, arg0, arg1, arg2, arg3, arg4, arg5, > > + arg6, &res); > > + > > + return res.a0; > > +} > > + > > +static int scmi_smc_conduit_method(struct device_node *np) { > > + const char *method; > > + > > + if (invoke_scmi_smc_fn) > > + return 0; > > + > > + if (of_property_read_string(np, "method", &method)) > > + return -ENXIO; > > + > > + if (!strcmp("hvc", method)) { > > + invoke_scmi_smc_fn = __invoke_scmi_fn_hvc; > > + } else if (!strcmp("smc", method)) { > > + invoke_scmi_smc_fn = __invoke_scmi_fn_smc; > > + } else { > > + pr_warn("invalid \"method\" property: %s\n", method); > > + return -EINVAL; > > + } > > + > > + return 0; > > +} > > + > > You don't the above functions ok > > [...] > > > + > > + np = of_find_node_by_path("/psci"); > > + if (!np) { > > + dev_err(dev, "Not able to find /psci node\n"); > > + return -ENODEV; > > + } > > No need for this as mentioned below. ok > > > + > > + ret = scmi_smc_conduit_method(np); > > Just use arm_smccc_1_1_get_conduit if you want to get the conduit which I > don't think you need. You can just use arm_smccc_1_1_invoke() directly. Fix in v3. I'll post v3 after we have an agreement on whether we need a new compatible string arm,scmi-smc and the prot_id introduced in chan_setup. Thanks, Peng. > > -- > Regards, > Sudeep ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-02-14 1:04 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-02-13 3:58 [PATCH V2 0/2] firmware: arm_scmi: add smc/hvc transports support peng.fan 2020-02-13 3:58 ` [PATCH V2 1/2] dt-bindings: arm: arm,scmi: add smc/hvc transports peng.fan 2020-02-13 10:54 ` Sudeep Holla 2020-02-14 0:59 ` Peng Fan 2020-02-13 3:58 ` [PATCH V2 2/2] firmware: arm_scmi: " peng.fan 2020-02-13 11:04 ` Sudeep Holla 2020-02-14 1:04 ` Peng Fan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).