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

[Sudeep Holla <sudeep.holla@arm.com>]

On Wed, Apr 01, 2020 at 09:14:36AM +0000, Peng Fan wrote:
> > 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.
>

No worries, I should have seen this during initial review, just missed
few trivial things.

--
Regards,
Sudeep