Messages in this thread | | | Date | Wed, 15 Apr 2020 14:16:12 +0100 | From | Sudeep Holla <> | Subject | Re: [PATCH V5 2/2] firmware: arm_scmi: add smc/hvc transport |
| |
On Wed, Apr 15, 2020 at 12:58:58PM +0200, Etienne Carriere wrote: > Hello Peng, > > I have 2 comments on this change. The main is about using > arm_smccc_1_1_invoke(). Below some details and I added comments > inside you patch. The second of on SMC return value, see my > comment in your patch below. > > About arm_smccc_1_1_invoke(), this functon currently relies on PSCI > driver to define a conduit method but SCMI agent driver does not > mandate CONFIG_PSCI to be enable. >
Yes this was discussed and it is done so deliberately. I have added the build dependency when I merged the patch. There's no dependency on CONFIG_PSCI.
> Could you add an optional "method" property for "arm,scmi-smc" for platforms > willing to not rely on PSCI Linux driver? If no property "method" is > defined in the FDT, invocation relies on arm_smccc_1_1_invoke(). >
Nope, we don't want mixture here. Why is the system not using PSCI/SMCCC ?
> "method" naming mimics what is done in the OP-TEE driver (drivers/tee/optee/). > Here is a proposal for the documenting property "method" in > Documentation/arm,scmi.txt: > > - method : "smc" or "hvc" > Optional property defining the conduit method for to be used > for invoking the SCMI server in secure world. > "smc" states instruction SMC #0 is used whereas "hvc" states > instruction HVC #0 is used. > >
It was rejected, you can try your luck with OPTEE :) We will just use the system conduit here with SCMI for SMC/HVC transport. Details in previous version of the patch.
[...]
> > +struct scmi_smc { > > + struct scmi_chan_info *cinfo; > > + struct scmi_shared_mem __iomem *shmem; > > + u32 func_id; > > +}; > > Add here a field for the secure world invocation function handler: > > scmi_arm_smccc_invoke_fn *invoke_fn; >
As stated not needed if we use arm_smccc_1_1_invoke()
[...]
> > The SCMI server is likely not to return a errno compliant value. > > SMCCC specification states that unsupported function IDs should return signed > extended -1. I suggest to change the return above with: > > return res.a0 == ~0 ? -EINVAL : 0; >
I need to check that.
-- Regards, Sudeep
| |