Re: [PATCH 0/6] mailbox: arm_mhu: add support to use in doorbell mode

From: Sudeep Holla
Date: Thu Jun 06 2019 - 08:55:52 EST


On Wed, Jun 05, 2019 at 07:51:12PM -0500, Jassi Brar wrote:
> On Wed, Jun 5, 2019 at 2:46 PM Mark Brown <broonie@xxxxxxxxxx> wrote:
> >
> > On Tue, Jun 04, 2019 at 10:44:24AM +0100, Sudeep Holla wrote:
> > > On Mon, Jun 03, 2019 at 08:39:46PM +0100, Mark Brown wrote:
> >
> > >
> > > > It feels like the issues with sharing access to the hardware and with the
> > > > API for talking to doorbell hardware are getting tied together and
> > > > confusing things. But like I say I might be missing something here.
> >
> > ...
> >
> > > So what I am trying to convey here is MHU controller hardware can be
> > > used choosing one of the different transport protocols available and
> > > that's platform choice based on the use-case.
> >
> > > The driver in the kernel should identify the same from the firmware/DT
> > > and configure it appropriately.
> >
> > > It may get inefficient and sometime impossible to address all use-case
> > > if we stick to one transport protocol in the driver and try to build
> > > an abstraction on top to use in different transport mode.
> >
> > Right, what I was trying to get at was that it feels like the discussion
> > is getting wrapped up in the specifics of the MHU rather than
> > representing this sort of controller with multiple modes in the
> > framework.
> >
> Usually when a controller could be used in more than one way, we
> implement the more generic usecase. And that's what was done for MHU.

That's debatable and we have done that so extensively so far.
So what I am saying is to implement different modes not just one so that
as many use-case are addressed.

> Implementing doorbell scheme would have disallowed mhu platforms that
> don't have any shmem between the endpoints. Now such platforms could
> use 32bits registers to pass/get data. Meanwhile doorbells could be
> emulated in client code.
> Also, next version of MHU has many (100?) such 32bit registers per
> interrupt. Clearly those are not meant to be seen as 3200 doorbells,
> but as message passing windows. (or maybe that is an almost different
> controller because of the differences)
>

I disagree. It's configurable and vendors can just choose 2 instead of
100s as you mentioned based on the use-case and needs. So we will still
need the same there.

> BTW, this is not going to be the end of SCMI troubles (I believe
> that's what his client is). SCMI will eventually have to be broken up
> in layers (protocol and transport) for many legit platforms to use it.
> That is mbox_send_message() will have to be replaced by, say,
> platform_mbox_send() in drivers/firmware/arm_scmi/driver.c OR the
> platforms have to have shmem and each mailbox controller driver (that
> could ever be used under scmi) will have to implement "doorbell
> emulation" mode. That is the reason I am not letting the way paved for
> such emulations.
>

While I don't dislike or disagree with separate transport in SCMI which
I have invested time and realised that I will duplicate mailbox framework
at the end. So I am against it only because of duplication and extra
layer of indirection which has performance impact(we have this seen in
sched governor for DVFS). So idea wise, it's good and I don't disagree
with practically seen performance impact. Hence I thought it's sane to
do something I am proposing. It also avoids coming up with virtual DT
nodes for this layer of abstract which I am completely against.

--
Regards,
Sudeep