RE: [PATCH V4 3/5] mailbox: imx: add imx mu support

[A.s. Dong]

Hi Jassi,

> -----Original Message-----
> From: Jassi Brar [mailto:jassisinghbrar@xxxxxxxxx]
> Sent: Wednesday, July 11, 2018 6:44 PM
> To: A.s. Dong <aisheng.dong@xxxxxxx>
> Cc: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>; linux-arm-
> kernel@xxxxxxxxxxxxxxxxxxx; dongas86@xxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx>; dl-
> linux-imx <linux-imx@xxxxxxx>; kernel@xxxxxxxxxxxxxx; Fabio Estevam
> <fabio.estevam@xxxxxxx>; shawnguo@xxxxxxxxxx
> Subject: Re: [PATCH V4 3/5] mailbox: imx: add imx mu support
> 
> On Wed, Jul 11, 2018 at 4:07 PM, A.s. Dong <aisheng.dong@xxxxxxx> wrote:
> >
> > > -----Original Message-----
> > > From: Sascha Hauer [mailto:s.hauer@xxxxxxxxxxxxxx]
> > > Sent: Wednesday, July 11, 2018 3:55 PM
> > > To: A.s. Dong <aisheng.dong@xxxxxxx>
> > > Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; dongas86@xxxxxxxxx; Jassi
> > > Brar <jassisinghbrar@xxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx;
> > > Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx>; dl-linux-imx
> > > <linux-imx@xxxxxxx>; kernel@xxxxxxxxxxxxxx; Fabio Estevam
> > > <fabio.estevam@xxxxxxx>; shawnguo@xxxxxxxxxx
> > > 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:
> > > > Hi Sascha,
> > > >
> > > > > -----Original Message-----
> > > > > From: Sascha Hauer [mailto:s.hauer@xxxxxxxxxxxxxx]
> > > > > Sent: Tuesday, July 10, 2018 10:20 PM
> > > > > To: A.s. Dong <aisheng.dong@xxxxxxx>
> > > > > Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; dongas86@xxxxxxxxx;
> > > > > Jassi Brar <jassisinghbrar@xxxxxxxxx>;
> > > > > linux-kernel@xxxxxxxxxxxxxxx; Oleksij Rempel
> > > > > <o.rempel@xxxxxxxxxxxxxx>; dl-linux-imx <linux-imx@xxxxxxx>;
> > > > > kernel@xxxxxxxxxxxxxx; Fabio Estevam <fabio.estevam@xxxxxxx>;
> > > > > shawnguo@xxxxxxxxxx
> > > > > 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:
> > > > > > 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.
> > > > > 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?
> > > 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.
> > 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);
	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);

        return true;
}
It is much specific to SCU.
Do you think such peek_data using for sync receiving mode is ok?

Regards
Dong Aisheng

> Cheers!