RE: [PATCH 1/4] firmware: arm_scmi: Make mutex channel specific

[Peng Fan <peng.fan@nxp.com>]

Hi Sudeep,

> Subject: [PATCH 1/4] firmware: arm_scmi: Make mutex channel specific
> 
> In order to support multiple SMC/HVC transport channels with associated
> shared memory, 

Does this mean each channel will have its own shared memory? Or
All channels share the same shared memory?

it is better to maintain the mutex per channel instead of
> existing global one.

If all channels shared the same memory, use per channel mutex lock
will not be able to prevent other channels accessing shared memory
at the same time.  

Thanks,
Peng.
> 
> Move the smc_mutex into the scmi_smc structure and also rename it to
> shmem_lock which is more appropriate for it's use.
> 
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  drivers/firmware/arm_scmi/smc.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/firmware/arm_scmi/smc.c
> b/drivers/firmware/arm_scmi/smc.c index 336168e40f49..6dc8a88cc91b
> 100644
> --- a/drivers/firmware/arm_scmi/smc.c
> +++ b/drivers/firmware/arm_scmi/smc.c
> @@ -27,11 +27,10 @@
>  struct scmi_smc {
>  	struct scmi_chan_info *cinfo;
>  	struct scmi_shared_mem __iomem *shmem;
> +	struct mutex shmem_lock;
>  	u32 func_id;
>  };
> 
> -static DEFINE_MUTEX(smc_mutex);
> -
>  static bool smc_chan_available(struct device *dev, int idx)  {
>  	return true;
> @@ -78,6 +77,7 @@ static int smc_chan_setup(struct scmi_chan_info *cinfo,
> struct device *dev,
> 
>  	scmi_info->func_id = func_id;
>  	scmi_info->cinfo = cinfo;
> +	mutex_init(&scmi_info->shmem_lock);
>  	cinfo->transport_info = scmi_info;
> 
>  	return 0;
> @@ -102,14 +102,14 @@ static int smc_send_message(struct
> scmi_chan_info *cinfo,
>  	struct scmi_smc *scmi_info = cinfo->transport_info;
>  	struct arm_smccc_res res;
> 
> -	mutex_lock(&smc_mutex);
> +	mutex_lock(&scmi_info->shmem_lock);
> 
>  	shmem_tx_prepare(scmi_info->shmem, xfer);
> 
>  	arm_smccc_1_1_invoke(scmi_info->func_id, 0, 0, 0, 0, 0, 0, 0, &res);
>  	scmi_rx_callback(scmi_info->cinfo,
> shmem_read_header(scmi_info->shmem));
> 
> -	mutex_unlock(&smc_mutex);
> +	mutex_unlock(&scmi_info->shmem_lock);
> 
>  	return res.a0;
>  }
> --
> 2.17.1