On Fri, Jul 27, 2018 at 9:32 AM, A.s. Dong [off-list ref] wrote:

quoted

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!

This looks like reasonable to me, theoretically.
Just not sure whether it's necessary to support it because we probably will never use
like that in reality, then it might become meaningless complicity introduced
and error prone.

It _is_ necessary to write controller driver independent of client drivers.

And AFAIK ARM MHU is doing the same way as MU which looks like also unidirectional channel.
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0515b/CHDGBGIF.html
drivers/mailbox/arm_mhu.c

MHU driver is doing exactly what the data sheet says. I know because I
wrote the driver.

quoted

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.

This is one of the recommended ways designed in HW reference manual and it
allows to send frame information up to 4 words without using shared memory.
SCU just follows it, so it's hard to believe it's a bad design.

quoted

#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.

It may work If SCU protocol data length is fixed to 4 words. However, it's length
is flexible for different SVC service. That means this binding won't work for SCU.
And it will introduce much complexities during the implementation.

Instead, we're using polling mode for both TX/RX and the data size is stored in
the msg header and sending msgs using all 4 data tx/rx registers as a channel fifo.

You first give me the "Passing short messages" usecase (quite a bad
one) and ask how it can work. When I give you a solution, you
effectively say "well, my usecase isn't even that". I feel I just
wasted my time.

quoted

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.

First of all, really appreciate for your suggestions.
It may be hard to find a common binding with the same mbox-cells.
It looks like we just need a property is distinguish how the Mailbox is used
In one channel or multi channel mode.

I get the idea you were ready to see the code merged in the coming
window and be done with it. And now it feels lazy to change the code.
I am sorry, but I can't allow controller drivers "emulating" some mode
for a client driver. That is moving a part of client code into the
controller driver.

Directly checking mbox-cells seems the most easy way and it is already Acked
by Rob. Can't this way be Okay to you?

Rob is indeed far better than I am. But he also has to look into 100
other subsystems, whereas I am only looking into mailbox. I have time
to dig into your datasheets. I believe Rob will point out anything
wrong I suggest.

BTW, the submitted driver doesn't even support the SCU usecase. Why
the bindings?