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

From: Sudeep Holla <sudeep.holla@arm.com>
To: Jassi Brar <jassisinghbrar@gmail.com>
Cc: Arnd Bergmann <arnd@arndb.de>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Rob Herring <robh+dt@kernel.org>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	Mark Brown <broonie@kernel.org>,
	Cristian Marussi <cristian.marussi@arm.com>,
	Sudeep Holla <sudeep.holla@arm.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 0/6] mailbox: arm_mhu: add support to use in doorbell mode
Date: Fri, 31 May 2019 17:53:26 +0100	[thread overview]
Message-ID: <20190531165326.GA18115@e107155-lin> (raw)
In-Reply-To: <CABb+yY1u5zdocgV=HhQcHWQa_R7ArtFqndU5_T=NsPHJ=jwseA@mail.gmail.com>

On Fri, May 31, 2019 at 11:21:08AM -0500, Jassi Brar wrote:
> Hi Sudeep,
>
> On Fri, May 31, 2019 at 9:33 AM Sudeep Holla <sudeep.holla@arm.com> wrote:
> >
> > This is my another attempt to extend mailbox framework to support
> > doorbell mode mailbox hardware. It also adds doorbell support to ARM
> > MHU driver.
> >
> Nothing has really changed since the last time we discussed many months ago.
> MHU remains same, and so are my points.
>

Yes, I understand your concern.

But as mentioned in the cover letter I did try the suggestions and have
detailed reasoning why that's still an issue. In short I ended up
re-inventing mailbox framework with all the queuing and similar APIs
for this. Worse, we can't even add an extra node for that in DT to
describe that. It can't be simple shim as we need to allow multiple
users to access one physical channel at a time. We have use case
where we can this for CPU DVFS fast switching in scheduler context.

> >
> > Brief hardware description about MHU
> > ====================================
> >
> > For each of the interrupt signals, the MHU drives the signal using a 32-bit
> > register, with all 32 bits logically ORed together. The MHU provides a set of
> > registers to enable software to SET, CLEAR, and check the STATUS of each of
> > the bits of this register independently. The use of 32 bits for each interrupt
> > line enables software to provide more information about the source of the
> > interrupt.
> >
> "32 bits for each interrupt line"  =>  32 virtual channels or 32bit
> data over one physical channel. And that is how the driver is
> currently written.
>

Yes, I understand.

> > For example, each bit of the register can be associated with a type
> > of event that can contribute to raising the interrupt.
> >
> Sure there can be a usecase where each bit is associated with an event
> (virtual channel). As you said, this is just one example of how MHU
> can be used. There are other ways MHU is used already.
>

Agreed and I am not saying that's incorrect or asking to remove
support for that. Future versions of MHU are making it more complex
and the specification classify the 3 modes of transport protocol in which
the new controller can be configured and used.

The choice is left to platform based on use case to get best/optimal
results. That's the reason I am trying to convince you that we need to
support all those configurations/transport protocols in the Linux
mailbox controller driver. As you mention one use-case of MHU, the word
transfer with 2^32 - 1 options is optimal for IoT use-case where as
doorbell mode is optimal for heavy payloads. The newer versions of
MHU are designed keeping both configurations in mind and the same is
suggested by h/w teams to
various vendors.

Arnd has similar suggestions(something like patch 1) to deal with such
configurations/transport protocols. Please note I am referring to
different transport protocols and not message protocols(SCPI/SCMI fall
under this category)

> Patch-2/6 looks good though. I will pick that up.
>

Thanks.

--
Regards,
Sudeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel