On Wed, Jul 11, 2018 at 6:28 PM, A.s. Dong [off-list ref] wrote:

Hi Jassi,

quoted

-----Original Message-----
From: Jassi Brar [mailto:jassisinghbrar at gmail.com]
Sent: Wednesday, July 11, 2018 6:44 PM
To: A.s. Dong <aisheng.dong@nxp.com>
Cc: Sascha Hauer <s.hauer@pengutronix.de>; linux-arm-
kernel at lists.infradead.org; dongas86 at gmail.com; linux-
kernel at vger.kernel.org; Oleksij Rempel [off-list ref]; dl-
linux-imx [off-list ref]; kernel at pengutronix.de; Fabio Estevam
[off-list ref]; shawnguo at kernel.org
Subject: Re: [PATCH V4 3/5] mailbox: imx: add imx mu support

On Wed, Jul 11, 2018 at 4:07 PM, A.s. Dong [off-list ref] wrote:

quoted

quoted

-----Original Message-----
From: Sascha Hauer [mailto:s.hauer at pengutronix.de]
Sent: Wednesday, July 11, 2018 3:55 PM
To: A.s. Dong <aisheng.dong@nxp.com>
Cc: linux-arm-kernel at lists.infradead.org; dongas86 at gmail.com; Jassi
Brar [off-list ref]; linux-kernel at vger.kernel.org;
Oleksij Rempel [off-list ref]; dl-linux-imx
[off-list ref]; kernel at pengutronix.de; Fabio Estevam
[off-list ref]; shawnguo at kernel.org
Subject: Re: [PATCH V4 3/5] mailbox: imx: add imx mu support

On Wed, Jul 11, 2018 at 07:29:38AM +0000, A.s. Dong wrote:

quoted

Hi Sascha,

quoted

-----Original Message-----
From: Sascha Hauer [mailto:s.hauer at pengutronix.de]
Sent: Tuesday, July 10, 2018 10:20 PM
To: A.s. Dong <aisheng.dong@nxp.com>
Cc: linux-arm-kernel at lists.infradead.org; dongas86 at gmail.com;
Jassi Brar [off-list ref];
linux-kernel at vger.kernel.org; Oleksij Rempel
[off-list ref]; dl-linux-imx [off-list ref];
kernel at pengutronix.de; Fabio Estevam [off-list ref];
shawnguo at kernel.org
Subject: Re: [PATCH V4 3/5] mailbox: imx: add imx mu support

Hi,

On Sun, Jul 08, 2018 at 10:56:55PM +0800, Dong Aisheng wrote:

quoted

This is used for i.MX multi core communication.
e.g. A core to SCU firmware(M core) on MX8.

Tx is using polling mode while Rx is interrupt driven and
schedule a hrtimer to receive remain words if have more than
4 words.

You told us that using interrupts is not possible due to
miserable performance, we then provided you a way with which you

could poll.

quoted

quoted

quoted

quoted

Why are you using interrupts now?

Because mailbox framework does not support sync rx now, I think we
do not need to wait for that feature done first as it's
independent and separate features of framework.

You can wait forever for this feature, nobody will add it for you.
It's up to you to add support for that feature. Who else should add this

feature if not you?

quoted

quoted

And when will you add that feature if not now when you actually need it?
It is common practice that you adjust the frameworks to your needs
rather than working around them.

I'm willing to add it. Just because you said Jassi already had the
idea on how to Implement it and does not add much complexity. So I just

want to see his patches.

quoted

But if he did not work on it, I can also help on it.

I am not much aware of the history of this conversation... but it seems you
need to make use of mbox_chan_ops.peek_data().

If not that, please let me know the requirement.

Thanks for the suggestion.
It looks to me may work.

From the definition, it seems it's used to pull data from remote side.
/**
 * mbox_client_peek_data - A way for client driver to pull data
 *                      received from remote by the controller.
 * @chan: Mailbox channel assigned to this client.
 *
 * A poke to controller driver for any received data.
 * The data is actually passed onto client via the
 * mbox_chan_received_data()
 * The call can be made from atomic context, so the controller's
 * implementation of peek_data() must not sleep.
 *
 * Return: True, if controller has, and is going to push after this,
 *          some data.
 *         False, if controller doesn't have any data to be read.
 */
bool mbox_client_peek_data(struct mbox_chan *chan)
{
        if (chan->mbox->ops->peek_data)
                return chan->mbox->ops->peek_data(chan);

        return false;
}
EXPORT_SYMBOL_GPL(mbox_client_peek_data);
But it seems most users in kernel simply implement it as a data available
Checking rather than receiving it.
See:
drivers/mailbox/ti-msgmgr.c
drivers/mailbox/mailbox-altera.c

Only bcm uses it to receive data.
drivers/mailbox/bcm-flexrm-mailbox.c

For our requirement, we want to implement sync receiving protocol like:
Sc_call_rpc()
{
        mbox_send_message(chan, msg)
        If (!no_resp)
                // rx also stored in msg
                mbox_receive_msg_in_polling(chan, msg);
        mbox_client_txdone();
}

If using peek_data, it can be:
Sc_call_rpc()
{
        mbox_send_message(chan, msg)
        If (!no_resp)
                // rx also stored in msg
                Mbox_client_peek_data(chan);

Yes, and you may want to loop for a certain amount of time if peek
keeps returning false.

        mbox_client_txdone();
}

And for mu controller driver .peek_data():
imx_mu_peek_data(chan)
{
        // get first word and parse data size
        imx_mu_receive_msg(&mu->chans, 0, mu->msg);

        raw_data = (u8 *)mu->msg;
        size = raw_data[1];

        // receive rest of them
        for (i = 1; i < size; i++) {
                ret = imx_mu_receive_msg(&mu->chans, i % 4, mu->msg + i);
                if (ret)
                        return false;
        }

        mbox_chan_received_data(&mu->chans, (void *)mu->msg);

Not sure how your controller works. But the peek() callback only
_checks_ if there is some data available to be read. Please note that
peek() can not sleep.
So if the data fetching doesn't sleep you can do that here, otherwise
peek has to schedule the actual fetching of data from remote and
providing to the client via mbox_chan_received_data.

-jassi