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

From: Peng Fan <peng.fan@nxp.com>
To: Sudeep Holla <sudeep.holla@arm.com>
Cc: "linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"f.fainelli@gmail.com" <f.fainelli@gmail.com>,
	dl-linux-imx <linux-imx@nxp.com>
Subject: RE: [PATCH 1/4] firmware: arm_scmi: Make mutex channel specific
Date: Wed, 1 Apr 2020 09:14:36 +0000	[thread overview]
Message-ID: <AM0PR04MB4481E4CC4FA7A55488E7663988C90@AM0PR04MB4481.eurprd04.prod.outlook.com> (raw)
In-Reply-To: <20200401091208.GB3954@bogus>

> Subject: Re: [PATCH 1/4] firmware: arm_scmi: Make mutex channel specific
> 
> On Wed, Apr 01, 2020 at 01:12:37AM +0000, Peng Fan wrote:
> > 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 depends on platform firmware and DT. If there is only one shmem at the top
> level scmi node, all share that single channel. If some/all protocols have their
> own channel, they there must be shmem entry in the corresponding child
> node.
> 
> > 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.
> >
> 
> No we don't create channel per protocol. If they share, we just share the
> channel pointer. Look at:
> 
>        if (!info->desc->ops->chan_available(dev, idx)) {
>                 cinfo = idr_find(idr, SCMI_PROTOCOL_BASE);
>                 if (unlikely(!cinfo)) /* Possible only if platform has no Rx */
>                         return -EINVAL;
>                 goto idr_alloc;
>         }
> 
> If a protocol doesn't have a dedicated channel, we just assign the base
> protocol channel to it. We don't call chan_setup at all on that channel.
> Your patch assumed so but the core driver never did that.
> 
> Hope this clarifies you doubt.

Yes. Thanks for the explainaiton.

Thanks,
Peng.

> 
> --
> Regards,
> Sudeep