[PATCH v7 3/6] dt-bindings: mailbox: imx-mu: add generic MU channel support

Hi Oleksij & Sascha,

-----Original Message-----
From: Jassi Brar [mailto:jassisinghbrar at gmail.com]
Sent: Thursday, July 26, 2018 11:44 PM
To: A.s. Dong <aisheng.dong@nxp.com>
Cc: Oleksij Rempel <o.rempel@pengutronix.de>; Shawn Guo
[off-list ref]; Fabio Estevam [off-list ref]; Rob
Herring [off-list ref]; Mark Rutland [off-list ref];
Vladimir Zapolskiy [off-list ref]; , Sascha Hauer
[off-list ref]; , linux-arm-kernel at lists.infradead.org, linux-
mediatek at lists.infradead.org, srv_heupstream <linux-arm-
kernel at lists.infradead.org>; Devicetree List [off-list ref];
dl-linux-imx [off-list ref]
Subject: Re: [PATCH v7 3/6] dt-bindings: mailbox: imx-mu: add generic MU
channel support

On Thu, Jul 26, 2018 at 5:21 PM, A.s. Dong [off-list ref] wrote:

quoted

quoted

quoted

Each MU has four pairs of rx/tx data register with four rx/tx
interrupts which can also be used as a separate channel.

So the hardware actually supports 4 channels.

quoted

-- #mbox-cells:  Must be 0. Number of cells in a mailbox
+- #mbox-cells:  Must be:
+               0 - for single channel mode. i.MX8* SCU protocol specific.
+               1 - for multichannel (generic) mode.
+

No, please.
DT bindings should reflect the real hardware, and not the software
mode we want the driver to work in.
Please define mbox-cells=1  and have the i.MX8* platform always ask
for channel-0.

The reality is that MU hardware does not define channels in reference

manual.

quoted

However, it does have four separate data tx/rx register which can be
used as 'virtual' channels which is supported by this current driver.

See below HW description from the reference manual:
For messaging, the MU has four, 32-bit write-only transmit registers
and four, 32-bit read-only receive registers on the Processor B and
Processor A-sides. These registers are used for sending messages to each

other.

quoted

For a while please forget the protocol(user) level usage, and consider only
what your h/w is.

MU has 4 pairs of TX_Reg + TX_IRQ, and 4 pairs of RX_Reg + RX_IRQ.
(MU also has 4 "doorbell" type channels that it calls GP, but those are not
managed here, so lets not worry atm).

By definition, a mailbox channel is simply a signal, optionally with data
attached. So, MU has 4 TX and 4 RX channels.

The MU driver should populate 8 unidirectional (4 Tx and 4 RX) channels and
set each tx/rx operation to trigger the corresponding interrupt. This is not my
whim, this is how the controller is!

The SCU is poorly implemented as it ignores 3 irqs and club all 4 register
together (there are many other cons of this approach but lets not get into
that).
Personally, I would push-back on such a bad design. But if you claim you have
no choice but to support SCU as such, the work around could be simpler than
defining a new "scu mode" altogether.

#mbox-cells:  Must be 3.
                      First cell is 1 for TX and 0 for RX channel
                      Second cell is index of the channel [0,1,2 or 3]
                      Third cell is 1 if the channel triggers an IRQ,
0 if not. That is ACR.RIE/TIE bits are set or not.

Normal clients would always request a channel with irqs enabled.
The SCU client would request all 8 channels -- TX/RX[0,1,2] with irqs disabled,
TX/RX[3] with irqs enabled. And SCU will read/write 4 word data over 4 rx/tx
channels, instead of the virtually concocted one.

Sascha & Oleksij,
Do you have any comments on this new approach suggested by Jassi?
This looks like may work for M4 case.

But I'm still discussing with Jassi on whether it's suitable for SCU as well?
As that means we drop the MU Hardware capability to send multi words
in one time but send one word on each channel one time which seems like
less efficiency and more complexity introduced for SCU case.

Regards
Dong Aisheng

quoted

short messages
Transmit register(s) can be used to pass short messages from one to
four words in length. For example, when a four-word message is
desired, only one of the registers needs to have its corresponding
interrupt enable bit set at the receiver side; the message?s first
three words are written to the registers whose interrupt is masked,
and the fourth word is written to the other register (which triggers an

interrupt at the receiver side).

quoted

The reference manual is at here: (Chapter 42 Messaging Unit (MU)

https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fw
ww

quoted

.nxp.com%2Fdocs%2Fen%2Freference-

manual%2FIMX6ULRM.pdf&data=02%7C0

quoted

1%7Caisheng.dong%40nxp.com%7Cef349d7c9caf46c4260008d5f30e9ef5%7C6
86ea1

quoted

d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636682166494860332&sdat
a=54rE

quoted

iDm%2BGD6EY8NE64ck1LXVGoUCtWYrHXPoWqcujUo%3D&reserved=0

quoted

And SCU firmware does use MU in above way specified by reference

manual.

quoted

Even from HW point of view, it's still one channel only mailbox.

Please realise that any manual is written by a mere mortal afterall.
How the controller works is set in stone, but how the controller can be
used ... is just someones suggestion.

The approach I suggest above, conforms to the api and prevents a provider
dancing to the tunes of a user.