On Thu, Jul 09, 2020 at 02:28:10PM +0100, Paul Murphy wrote: > > On 7/9/20 13:23, Daniele Alessandrelli wrote: > > Hi Sudeep, > > > > Thanks for your review. > > > > On Wed, 2020-07-08 at 21:34 +0100, Sudeep Holla wrote: > > > On Tue, Jun 16, 2020 at 04:56:08PM +0100, Daniele Alessandrelli > > > wrote: > > > > From: Paul Murphy <paul.j.murphy@xxxxxxxxx> > > > > > > > > Keem Bay SoC has a ARM trusted firmware-based secure monitor which > > > > acts > > > > as the SCP for the purposes of power management over SCMI. > > > > > > > > This driver implements the transport layer for SCMI to function. > > > > > > > Please use the smc transport support in > > > driver/firmware/arm_scmi/smc.c > > > for this. You don't need mailbox support for SMC/HVC. Basically you > > > don't need this driver at all and you have everything you need to > > > support > > > what you want. > > > > > > Let me know if you face issues. > > > > > Sorry, we didn't know about the SMC transport support for SCMI. Looks > > like it was added only recently, while our driver was already developed > > and waiting to be upstreamed. > > > > I agree that we can drop this driver and switch to the SMC transport as > > you suggested, but I think we'll have to modify our bootloader SiP > > service slightly. Paul, can you elaborate? > > > > Just one question. > > In our patch, we pass the shared memory address as the second argument of > the SiP service, as it means we don't have to hardcode that in our firmware. > Sudeep, do you know if it was intentional in smc_send_message() to leave > that out? If we leave it out, we are requiring the secure monitor to > hardcode the shared memory address. > Please post a patch adding the address as 2nd parameter. Cc Peng Fan from NXP who is the original author of the file. I was also wondering why have I not added the address when I extended support for multiple channel/shmem with smc/hvc. One key point here is that firmware *must not* return INVALID PARAMETER and *must* ignore it. May be we can add a note while adding that the firmware can ignore that parameter if it supports only one channel && hardcoded in the firmware. It needs to be PA as obtained from the DT. Since it is custom SIP id, it needs to be well documented. -- Regards, Sudeep
