diff mbox series

[v8,4/4] mailbox: Add support for i.MX7D messaging unit

Message ID 20180731141146.10788-5-o.rempel@pengutronix.de (mailing list archive)
State New, archived
Headers show
Series add mailbox support for i.MX7D | expand

Commit Message

Oleksij Rempel July 31, 2018, 2:11 p.m. UTC
The Mailbox controller is able to send messages (up to 4 32 bit words)
between the endpoints.

This driver was tested using the mailbox-test driver sending messages
between the Cortex-A7 and the Cortex-M4.

Reviewed-by: Vladimir Zapolskiy <vz@mleia.com>
Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/mailbox/Kconfig       |   6 +
 drivers/mailbox/Makefile      |   2 +
 drivers/mailbox/imx-mailbox.c | 371 ++++++++++++++++++++++++++++++++++
 3 files changed, 379 insertions(+)
 create mode 100644 drivers/mailbox/imx-mailbox.c

Comments

Jassi Brar July 31, 2018, 4:21 p.m. UTC | #1
Hi Oleksij,

 A fast and fine turnaround! Mostly nits and a suggestion follows ...

On Tue, Jul 31, 2018 at 7:41 PM, Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> The Mailbox controller is able to send messages (up to 4 32 bit words)
> between the endpoints.
>
> This driver was tested using the mailbox-test driver sending messages
> between the Cortex-A7 and the Cortex-M4.
>
Do we need something MU specific here?
I mean MU doesn't really care if the endpoints are CA, CR or CM. So,
maybe just the two-line ritual intro to MU?


> +enum imx_mu_chan_type {
> +       IMX_MU_TYPE_TX,         /* Tx with FIFO */
> +       IMX_MU_TYPE_RX,         /* Rx with FIFO */
>
nit: its a register, not fifo


> +
> +static void imx_mu_txdb_work(struct work_struct *work)
> +{
> +       struct imx_mu_con_priv *cp =
> +                       container_of(work, struct imx_mu_con_priv, work);
> +       mbox_chan_txdone(cp->chan, 0);
> +}
>
The api assumes a controller have same type of channels. So we are
having to do this here.
I am not sure, but would declaring two mailbox controllers (one with
doorbells and the other data channels) work?
If not, at least we could use a tasklet instead of a work queue?


> +static bool imx_mu_last_tx_done(struct mbox_chan *chan)
> +{
> +       struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox);
> +       struct imx_mu_con_priv *cp = chan->con_priv;
> +
> +       /* test if transmit register is empty */
> +       return imx_mu_read(priv, IMX_MU_xSR) & IMX_MU_xSR_TEn(cp->idx);
> +}
> +
> +static int imx_mu_send_data(struct mbox_chan *chan, void *data)
> +{
> +       struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox);
> +       struct imx_mu_con_priv *cp = chan->con_priv;
> +       u32 *arg = data;
> +
> +       switch (cp->type) {
> +       case IMX_MU_TYPE_TX:
> +               if (!imx_mu_last_tx_done(chan))
> +                       return -EBUSY;
>
This test should be moved to imx_mu_startup().
The api won't ever call if last tx was not done, and so it doesn't
know how to recover from send_data() failure.


> +
> +static int imx_mu_startup(struct mbox_chan *chan)
> +{
> +       struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox);
> +       struct imx_mu_con_priv *cp = chan->con_priv;
> +       int ret;
> +
> +       if (cp->type == IMX_MU_TYPE_TXDB) {
> +               /* Tx doorbell don't have ACK support */
> +               INIT_WORK(&cp->work, imx_mu_txdb_work);
> +               return 0;
> +       }
> +
> +       cp->irq_desc = kasprintf(GFP_KERNEL, "imx_mu_chan[%i-%i]", cp->type,
> +                                cp->idx);
> +       if (!cp->irq_desc)
> +               return -ENOMEM;
>
Probably you won't do but still .... name of irq is but one channel
property. So you could set it in probe() and avoid creating another
situation when startup could fail.


> +       ret = request_irq(priv->irq, imx_mu_isr, IRQF_SHARED, cp->irq_desc,
> +                         chan);
> +       if (ret) {
> +               dev_err(priv->dev,
> +                       "Unable to acquire IRQ %d\n", priv->irq);
> +               return ret;
> +       }
> +
> +       switch (cp->type) {
> +               break;
>
Is this intentional?


> +
> +static void imx_mu_shutdown(struct mbox_chan *chan)
> +{
> +       struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox);
> +       struct imx_mu_con_priv *cp = chan->con_priv;
> +
> +       if (IMX_MU_TYPE_TXDB == cp->type)
>
nit: usually we do (cp->type == IMX_MU_TYPE_TXDB)


Thanks
Oleksij Rempel Aug. 1, 2018, 5:31 a.m. UTC | #2
On Tue, Jul 31, 2018 at 09:51:37PM +0530, Jassi Brar wrote:
> Hi Oleksij,
> 
>  A fast and fine turnaround! Mostly nits and a suggestion follows ...
> 
> On Tue, Jul 31, 2018 at 7:41 PM, Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> > The Mailbox controller is able to send messages (up to 4 32 bit words)
> > between the endpoints.
> >
> > This driver was tested using the mailbox-test driver sending messages
> > between the Cortex-A7 and the Cortex-M4.
> >
> Do we need something MU specific here?
> I mean MU doesn't really care if the endpoints are CA, CR or CM. So,
> maybe just the two-line ritual intro to MU?

Till now, every additional word about MU added more disagreement in this
topic. I prefer the reduced variant.

> 
> > +enum imx_mu_chan_type {
> > +       IMX_MU_TYPE_TX,         /* Tx with FIFO */
> > +       IMX_MU_TYPE_RX,         /* Rx with FIFO */
> >
> nit: its a register, not fifo

ok

> 
> > +
> > +static void imx_mu_txdb_work(struct work_struct *work)
> > +{
> > +       struct imx_mu_con_priv *cp =
> > +                       container_of(work, struct imx_mu_con_priv, work);
> > +       mbox_chan_txdone(cp->chan, 0);
> > +}
> >
> The api assumes a controller have same type of channels. So we are
> having to do this here.
> I am not sure, but would declaring two mailbox controllers (one with
> doorbells and the other data channels) work?

I was thinking about it and decided that this pain was not worth it.

> If not, at least we could use a tasklet instead of a work queue?

ok. probably it is better to fix the mailbox framework.... may be not right
now.

> 
> > +static bool imx_mu_last_tx_done(struct mbox_chan *chan)
> > +{
> > +       struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox);
> > +       struct imx_mu_con_priv *cp = chan->con_priv;
> > +
> > +       /* test if transmit register is empty */
> > +       return imx_mu_read(priv, IMX_MU_xSR) & IMX_MU_xSR_TEn(cp->idx);
> > +}
> > +
> > +static int imx_mu_send_data(struct mbox_chan *chan, void *data)
> > +{
> > +       struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox);
> > +       struct imx_mu_con_priv *cp = chan->con_priv;
> > +       u32 *arg = data;
> > +
> > +       switch (cp->type) {
> > +       case IMX_MU_TYPE_TX:
> > +               if (!imx_mu_last_tx_done(chan))
> > +                       return -EBUSY;
> >
> This test should be moved to imx_mu_startup().
> The api won't ever call if last tx was not done, and so it doesn't
> know how to recover from send_data() failure.

ok.

> 
> > +
> > +static int imx_mu_startup(struct mbox_chan *chan)
> > +{
> > +       struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox);
> > +       struct imx_mu_con_priv *cp = chan->con_priv;
> > +       int ret;
> > +
> > +       if (cp->type == IMX_MU_TYPE_TXDB) {
> > +               /* Tx doorbell don't have ACK support */
> > +               INIT_WORK(&cp->work, imx_mu_txdb_work);
> > +               return 0;
> > +       }
> > +
> > +       cp->irq_desc = kasprintf(GFP_KERNEL, "imx_mu_chan[%i-%i]", cp->type,
> > +                                cp->idx);
> > +       if (!cp->irq_desc)
> > +               return -ENOMEM;
> >
> Probably you won't do but still .... name of irq is but one channel
> property. So you could set it in probe() and avoid creating another
> situation when startup could fail.

You mean, fail on probe with no mem is better then fail on startup? Why?

> 
> > +       ret = request_irq(priv->irq, imx_mu_isr, IRQF_SHARED, cp->irq_desc,
> > +                         chan);
> > +       if (ret) {
> > +               dev_err(priv->dev,
> > +                       "Unable to acquire IRQ %d\n", priv->irq);
> > +               return ret;
> > +       }
> > +
> > +       switch (cp->type) {
> > +               break;
> >
> Is this intentional?

no. thx.

> 
> > +
> > +static void imx_mu_shutdown(struct mbox_chan *chan)
> > +{
> > +       struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox);
> > +       struct imx_mu_con_priv *cp = chan->con_priv;
> > +
> > +       if (IMX_MU_TYPE_TXDB == cp->type)
> >
> nit: usually we do (cp->type == IMX_MU_TYPE_TXDB)

why? I do it with simple reason:
this error will be detected by compiler
if (IMX_MU_TYPE_TXDB = cp->type)

this error will silently fail
if (cp->type = IMX_MU_TYPE_TXDB)
Jassi Brar Aug. 1, 2018, 7:02 a.m. UTC | #3
On Wed, Aug 1, 2018 at 11:01 AM, Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> On Tue, Jul 31, 2018 at 09:51:37PM +0530, Jassi Brar wrote:

>> > The Mailbox controller is able to send messages (up to 4 32 bit words)
>> > between the endpoints.
>> >
>> > This driver was tested using the mailbox-test driver sending messages
>> > between the Cortex-A7 and the Cortex-M4.
>> >
>> Do we need something MU specific here?
>> I mean MU doesn't really care if the endpoints are CA, CR or CM. So,
>> maybe just the two-line ritual intro to MU?
>
> Till now, every additional word about MU added more disagreement in this
> topic. I prefer the reduced variant.
>
OK, but we don't usually specify "tested on ..." but leave details of
the controller, in the new driver commit.


>> > +
>> > +static void imx_mu_txdb_work(struct work_struct *work)
>> > +{
>> > +       struct imx_mu_con_priv *cp =
>> > +                       container_of(work, struct imx_mu_con_priv, work);
>> > +       mbox_chan_txdone(cp->chan, 0);
>> > +}
>> >
>> The api assumes a controller have same type of channels. So we are
>> having to do this here.
>> I am not sure, but would declaring two mailbox controllers (one with
>> doorbells and the other data channels) work?
>
> I was thinking about it and decided that this pain was not worth it.
>
OK.

But just fyi, a common use of doorbell is to hint the master to take
actions like reset, suspend or expect some periodic broadcast or some
hotpath where we wouldn't want to sleep.
Another usecase is data transfer (where speed matters) via
doorbell+shmem, having to sleep after each packet is going to kill
performance.


>> If not, at least we could use a tasklet instead of a work queue?
>
> ok. probably it is better to fix the mailbox framework.... may be not right
> now.
>
No, the framework is fine. MU is the only controller that has two
types of channels.


>> > +
>> > +static int imx_mu_startup(struct mbox_chan *chan)
>> > +{
>> > +       struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox);
>> > +       struct imx_mu_con_priv *cp = chan->con_priv;
>> > +       int ret;
>> > +
>> > +       if (cp->type == IMX_MU_TYPE_TXDB) {
>> > +               /* Tx doorbell don't have ACK support */
>> > +               INIT_WORK(&cp->work, imx_mu_txdb_work);
>> > +               return 0;
>> > +       }
>> > +
>> > +       cp->irq_desc = kasprintf(GFP_KERNEL, "imx_mu_chan[%i-%i]", cp->type,
>> > +                                cp->idx);
>> > +       if (!cp->irq_desc)
>> > +               return -ENOMEM;
>> >
>> Probably you won't do but still .... name of irq is but one channel
>> property. So you could set it in probe() and avoid creating another
>> situation when startup could fail.
>
> You mean, fail on probe with no mem is better then fail on startup? Why?
>
Fail early :)
But if you just have irq_name[8] in the channel struct, it won't even
fail in probe. And is more consistent with other channel
initialisations..... you see name of irq doesn't change across channel
requests.

But OK, I am running out of energy and interest.


>> > +
>> > +static void imx_mu_shutdown(struct mbox_chan *chan)
>> > +{
>> > +       struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox);
>> > +       struct imx_mu_con_priv *cp = chan->con_priv;
>> > +
>> > +       if (IMX_MU_TYPE_TXDB == cp->type)
>> >
>> nit: usually we do (cp->type == IMX_MU_TYPE_TXDB)
>
> why? I do it with simple reason:
> this error will be detected by compiler
> if (IMX_MU_TYPE_TXDB = cp->type)
>
> this error will silently fail
> if (cp->type = IMX_MU_TYPE_TXDB)
>
Because it reads like
     if (2 is equal to the variable)
instead of
    if (the variable is equal to 2)

Most developers prefer latter. In fact you too, in startup()
So please atleast make it consistent either way.

Cheers!
Aisheng Dong Aug. 1, 2018, 7:44 a.m. UTC | #4
> -----Original Message-----
> From: Jassi Brar [mailto:jassisinghbrar@gmail.com]

[...]

> > +
> > +static void imx_mu_txdb_work(struct work_struct *work) {
> > +       struct imx_mu_con_priv *cp =
> > +                       container_of(work, struct imx_mu_con_priv, work);
> > +       mbox_chan_txdone(cp->chan, 0); }
> >
> The api assumes a controller have same type of channels. So we are having
> to do this here.
> I am not sure, but would declaring two mailbox controllers (one with
> doorbells and the other data channels) work?
> If not, at least we could use a tasklet instead of a work queue?
> 

I'm also a bit curious what the meaning of calling mbox_chan_txdone
for a doorbell mailbox?

Is there any recommended way to use it?

Regards
Dong Aisheng
Aisheng Dong Aug. 1, 2018, 7:52 a.m. UTC | #5
> -----Original Message-----
> From: Oleksij Rempel [mailto:o.rempel@pengutronix.de]
> Sent: Tuesday, July 31, 2018 10:12 PM
> To: Shawn Guo <shawnguo@kernel.org>; Fabio Estevam
> <fabio.estevam@nxp.com>; Rob Herring <robh+dt@kernel.org>; Mark
> Rutland <mark.rutland@arm.com>; A.s. Dong <aisheng.dong@nxp.com>;
> Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>; Jassi Brar
> <jassisinghbrar@gmail.com>
> Cc: Oleksij Rempel <o.rempel@pengutronix.de>; kernel@pengutronix.de;
> linux-arm-kernel@lists.infradead.org; devicetree@vger.kernel.org; dl-linux-
> imx <linux-imx@nxp.com>
> Subject: [PATCH v8 4/4] mailbox: Add support for i.MX7D messaging unit
> 
> The Mailbox controller is able to send messages (up to 4 32 bit words)
> between the endpoints.
> 

I'm not sure but is this still valid after change to multi channels mode?
We actually lost the HW capability to send messages up to 4 words
in the multi channels mode.
And such commit message probably will confuse people.
Worth a improve?

Otherwise this patch mostly is okay to me except a few bits already
pointed out by Jassi.

Regards
Dong Aisheng
Oleksij Rempel Aug. 1, 2018, 8 a.m. UTC | #6
On Wed, Aug 01, 2018 at 07:52:43AM +0000, A.s. Dong wrote:
> > -----Original Message-----
> > From: Oleksij Rempel [mailto:o.rempel@pengutronix.de]
> > Sent: Tuesday, July 31, 2018 10:12 PM
> > To: Shawn Guo <shawnguo@kernel.org>; Fabio Estevam
> > <fabio.estevam@nxp.com>; Rob Herring <robh+dt@kernel.org>; Mark
> > Rutland <mark.rutland@arm.com>; A.s. Dong <aisheng.dong@nxp.com>;
> > Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>; Jassi Brar
> > <jassisinghbrar@gmail.com>
> > Cc: Oleksij Rempel <o.rempel@pengutronix.de>; kernel@pengutronix.de;
> > linux-arm-kernel@lists.infradead.org; devicetree@vger.kernel.org; dl-linux-
> > imx <linux-imx@nxp.com>
> > Subject: [PATCH v8 4/4] mailbox: Add support for i.MX7D messaging unit
> > 
> > The Mailbox controller is able to send messages (up to 4 32 bit words)
> > between the endpoints.
> > 
> 
> I'm not sure but is this still valid after change to multi channels mode?
> We actually lost the HW capability to send messages up to 4 words
> in the multi channels mode.
> And such commit message probably will confuse people.
> Worth a improve?
> 
> Otherwise this patch mostly is okay to me except a few bits already
> pointed out by Jassi.

Ok, I schedule this task for Thursday or Friday.
Jassi Brar Aug. 1, 2018, 9:45 a.m. UTC | #7
On Wed, Aug 1, 2018 at 1:14 PM, A.s. Dong <aisheng.dong@nxp.com> wrote:
>> -----Original Message-----
>> From: Jassi Brar [mailto:jassisinghbrar@gmail.com]
>
> [...]
>
>> > +
>> > +static void imx_mu_txdb_work(struct work_struct *work) {
>> > +       struct imx_mu_con_priv *cp =
>> > +                       container_of(work, struct imx_mu_con_priv, work);
>> > +       mbox_chan_txdone(cp->chan, 0); }
>> >
>> The api assumes a controller have same type of channels. So we are having
>> to do this here.
>> I am not sure, but would declaring two mailbox controllers (one with
>> doorbells and the other data channels) work?
>> If not, at least we could use a tasklet instead of a work queue?
>>
>
> I'm also a bit curious what the meaning of calling mbox_chan_txdone
> for a doorbell mailbox?
>
> Is there any recommended way to use it?
>
The framework submits a message (a signal with or without data) for
controller to transmit. Obviously, it has to know when the transfer
completes (so that it can submit the next message).

Depending on the controller h/w design, there can be three ways.

a) Controller has some irq that fires when the signal is consumed by
the other end. Like MU does. So upon TX-IRQ, the controller driver
calls mbox_chan_txdone() to tell the framework that the last submitted
transfer has completed. This is indicated by setting only  txdone_irq
= true

b) Controller does not have any tx-irq, but it can read some register
to know the status. That is it has to be polled. The framework calls
last_tx_done() periodically to check the status. This is indicated by
setting only  txdone_poll = true.

c) Controller has neither irq, nor any status register. This is
indicated by setting only  txdone_irq = false and  txdone_poll =
false.   In this case, the protocol/client has to tell the framework
(using some protocol level indicator like some ack packet reception)
when the last submitted message was transferred, by calling
mbox_client_txdone()

A 'doorbell' is  (c) type channel -- just an irq raised at other end
without any ack irq or status bit to check. So, the client would
simply do
                mbox_send_message(chan, msg);
                mbox_client_txdone(chan, 0);
Aisheng Dong Aug. 2, 2018, 3:03 a.m. UTC | #8
> -----Original Message-----
> From: Jassi Brar [mailto:jassisinghbrar@gmail.com]
> Sent: Wednesday, August 1, 2018 5:45 PM

[...]

> >> > +
> >> > +static void imx_mu_txdb_work(struct work_struct *work) {
> >> > +       struct imx_mu_con_priv *cp =
> >> > +                       container_of(work, struct imx_mu_con_priv, work);
> >> > +       mbox_chan_txdone(cp->chan, 0); }
> >> >
> >> The api assumes a controller have same type of channels. So we are
> >> having to do this here.
> >> I am not sure, but would declaring two mailbox controllers (one with
> >> doorbells and the other data channels) work?
> >> If not, at least we could use a tasklet instead of a work queue?
> >>
> >
> > I'm also a bit curious what the meaning of calling mbox_chan_txdone
> > for a doorbell mailbox?
> >
> > Is there any recommended way to use it?
> >
> The framework submits a message (a signal with or without data) for
> controller to transmit. Obviously, it has to know when the transfer completes
> (so that it can submit the next message).
> 
> Depending on the controller h/w design, there can be three ways.
> 
> a) Controller has some irq that fires when the signal is consumed by the
> other end. Like MU does. So upon TX-IRQ, the controller driver calls
> mbox_chan_txdone() to tell the framework that the last submitted transfer
> has completed. This is indicated by setting only  txdone_irq = true
> 
> b) Controller does not have any tx-irq, but it can read some register to know
> the status. That is it has to be polled. The framework calls
> last_tx_done() periodically to check the status. This is indicated by setting
> only  txdone_poll = true.
> 
> c) Controller has neither irq, nor any status register. This is indicated by
> setting only  txdone_irq = false and  txdone_poll =
> false.   In this case, the protocol/client has to tell the framework
> (using some protocol level indicator like some ack packet reception) when
> the last submitted message was transferred, by calling
> mbox_client_txdone()
> 

Thanks for a detailed and clear explanation.

> A 'doorbell' is  (c) type channel -- just an irq raised at other end without any
> ack irq or status bit to check. So, the client would simply do
>                 mbox_send_message(chan, msg);
>                 mbox_client_txdone(chan, 0);

Just a bit wondering whether we need client to call mbox_client_txdone()
here as it actually does not know whether it's done. Not sure if it's better to
handle them in mailbox controller driver or framework.
(Oleksij seems do it in the controller driver in this patch)

And see mbox_client_txdone() definition, it seems it's only used by TXDONE_BY_ACK
case. Is the doorbell mailbox without IRQ or ACK support still suitable to call it to
notify the framework the transfer is done?

void mbox_client_txdone(struct mbox_chan *chan, int r)                                                                                                                                                              
{
        if (unlikely(!(chan->txdone_method & TXDONE_BY_ACK))) {
                dev_err(chan->mbox->dev, "Client can't run the TX ticker\n");                                                                                                                                       
                return;
        }

        tx_tick(chan, r);
}

For i.MX MU case, the controller is defined as TXDONE_BY_IRQ for all channels
Including TX doorbell. That's probably why we use mbox_chan_txdone to complete
It in controller driver. Slightly a bit strange.
Not sure if better idea to handle it in controller driver or framework.

Regards
Dong Aisheng
diff mbox series

Patch

diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index a2bb27446dce..79060ddc380d 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -15,6 +15,12 @@  config ARM_MHU
 	  The controller has 3 mailbox channels, the last of which can be
 	  used in Secure mode only.
 
+config IMX_MBOX
+	tristate "i.MX Mailbox"
+	depends on ARCH_MXC || COMPILE_TEST
+	help
+	  Mailbox implementation for i.MX Messaging Unit (MU).
+
 config PLATFORM_MHU
 	tristate "Platform MHU Mailbox"
 	depends on OF
diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
index cc23c3a43fcd..ba2fe1b6dd62 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -7,6 +7,8 @@  obj-$(CONFIG_MAILBOX_TEST)	+= mailbox-test.o
 
 obj-$(CONFIG_ARM_MHU)	+= arm_mhu.o
 
+obj-$(CONFIG_IMX_MBOX)	+= imx-mailbox.o
+
 obj-$(CONFIG_PLATFORM_MHU)	+= platform_mhu.o
 
 obj-$(CONFIG_PL320_MBOX)	+= pl320-ipc.o
diff --git a/drivers/mailbox/imx-mailbox.c b/drivers/mailbox/imx-mailbox.c
new file mode 100644
index 000000000000..6559a4695019
--- /dev/null
+++ b/drivers/mailbox/imx-mailbox.c
@@ -0,0 +1,371 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018 Pengutronix, Oleksij Rempel <o.rempel@pengutronix.de>
+ */
+
+#include <linux/clk.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/mailbox_controller.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/slab.h>
+
+/* Transmit Register */
+#define IMX_MU_xTRn(x)		(0x00 + 4 * (x))
+/* Receive Register */
+#define IMX_MU_xRRn(x)		(0x10 + 4 * (x))
+/* Status Register */
+#define IMX_MU_xSR		0x20
+#define IMX_MU_xSR_GIPn(x)	BIT(28 + (3 - (x)))
+#define IMX_MU_xSR_RFn(x)	BIT(24 + (3 - (x)))
+#define IMX_MU_xSR_TEn(x)	BIT(20 + (3 - (x)))
+#define IMX_MU_xSR_BRDIP	BIT(9)
+
+/* Control Register */
+#define IMX_MU_xCR		0x24
+/* General Purpose Interrupt Enable */
+#define IMX_MU_xCR_GIEn(x)	BIT(28 + (3 - (x)))
+/* Receive Interrupt Enable */
+#define IMX_MU_xCR_RIEn(x)	BIT(24 + (3 - (x)))
+/* Transmit Interrupt Enable */
+#define IMX_MU_xCR_TIEn(x)	BIT(20 + (3 - (x)))
+/* General Purpose Interrupt Request */
+#define IMX_MU_xCR_GIRn(x)	BIT(16 + (3 - (x)))
+
+#define IMX_MU_CHANS	16
+
+enum imx_mu_chan_type {
+	IMX_MU_TYPE_TX,		/* Tx with FIFO */
+	IMX_MU_TYPE_RX,		/* Rx with FIFO */
+	IMX_MU_TYPE_TXDB,	/* Tx doorbell */
+	IMX_MU_TYPE_RXDB,	/* Rx doorbell */
+};
+
+struct imx_mu_con_priv {
+	unsigned int		idx;
+	char			*irq_desc;
+	enum imx_mu_chan_type	type;
+	struct work_struct	work;
+	struct mbox_chan	*chan;
+};
+
+struct imx_mu_priv {
+	struct device		*dev;
+	void __iomem		*base;
+	spinlock_t		xcr_lock; /* control register lock */
+
+	struct mbox_controller	mbox;
+	struct mbox_chan	mbox_chans[IMX_MU_CHANS];
+
+	struct imx_mu_con_priv  con_priv[IMX_MU_CHANS];
+	struct clk		*clk;
+	int			irq;
+
+	bool			side_b;
+};
+
+static struct imx_mu_priv *to_imx_mu_priv(struct mbox_controller *mbox)
+{
+	return container_of(mbox, struct imx_mu_priv, mbox);
+}
+
+static void imx_mu_write(struct imx_mu_priv *priv, u32 val, u32 offs)
+{
+	iowrite32(val, priv->base + offs);
+}
+
+static u32 imx_mu_read(struct imx_mu_priv *priv, u32 offs)
+{
+	return ioread32(priv->base + offs);
+}
+
+static u32 imx_mu_xcr_rmw(struct imx_mu_priv *priv, u32 set, u32 clr)
+{
+	unsigned long flags;
+	u32 val;
+
+	spin_lock_irqsave(&priv->xcr_lock, flags);
+	val = imx_mu_read(priv, IMX_MU_xCR);
+	val &= ~clr;
+	val |= set;
+	imx_mu_write(priv, val, IMX_MU_xCR);
+	spin_unlock_irqrestore(&priv->xcr_lock, flags);
+
+	return val;
+}
+
+static void imx_mu_txdb_work(struct work_struct *work)
+{
+	struct imx_mu_con_priv *cp =
+			container_of(work, struct imx_mu_con_priv, work);
+	mbox_chan_txdone(cp->chan, 0);
+}
+
+static irqreturn_t imx_mu_isr(int irq, void *p)
+{
+	struct mbox_chan *chan = p;
+	struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox);
+	struct imx_mu_con_priv *cp = chan->con_priv;
+	u32 val, ctrl, dat;
+
+	ctrl = imx_mu_read(priv, IMX_MU_xCR);
+	val = imx_mu_read(priv, IMX_MU_xSR);
+
+	switch (cp->type) {
+	case IMX_MU_TYPE_TX:
+		val &= IMX_MU_xSR_TEn(cp->idx) &
+			(ctrl & IMX_MU_xCR_TIEn(cp->idx));
+		break;
+	case IMX_MU_TYPE_RX:
+		val &= IMX_MU_xSR_RFn(cp->idx) &
+			(ctrl & IMX_MU_xCR_RIEn(cp->idx));
+		break;
+	case IMX_MU_TYPE_RXDB:
+		val &= IMX_MU_xSR_GIPn(cp->idx) &
+			(ctrl & IMX_MU_xCR_GIEn(cp->idx));
+		break;
+	default:
+		break;
+	}
+
+	if (!val) {
+		return IRQ_NONE;
+	} else if (IMX_MU_xSR_TEn(cp->idx) == val) {
+		imx_mu_xcr_rmw(priv, 0, IMX_MU_xCR_TIEn(cp->idx));
+		mbox_chan_txdone(chan, 0);
+	} else if (IMX_MU_xSR_RFn(cp->idx) == val) {
+		dat = imx_mu_read(priv, IMX_MU_xRRn(cp->idx));
+		mbox_chan_received_data(chan, (void *)&dat);
+	} else if (IMX_MU_xSR_GIPn(cp->idx) == val) {
+		imx_mu_write(priv, IMX_MU_xSR_GIPn(cp->idx), IMX_MU_xSR);
+		mbox_chan_received_data(chan, NULL);
+	} else {
+		dev_warn_ratelimited(priv->dev, "Not handled interrupt\n");
+		return IRQ_NONE;
+	}
+
+	return IRQ_HANDLED;
+}
+
+static bool imx_mu_last_tx_done(struct mbox_chan *chan)
+{
+	struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox);
+	struct imx_mu_con_priv *cp = chan->con_priv;
+
+	/* test if transmit register is empty */
+	return imx_mu_read(priv, IMX_MU_xSR) & IMX_MU_xSR_TEn(cp->idx);
+}
+
+static int imx_mu_send_data(struct mbox_chan *chan, void *data)
+{
+	struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox);
+	struct imx_mu_con_priv *cp = chan->con_priv;
+	u32 *arg = data;
+
+	switch (cp->type) {
+	case IMX_MU_TYPE_TX:
+		if (!imx_mu_last_tx_done(chan))
+			return -EBUSY;
+		imx_mu_write(priv, *arg, IMX_MU_xTRn(cp->idx));
+		imx_mu_xcr_rmw(priv, IMX_MU_xCR_TIEn(cp->idx), 0);
+		break;
+	case IMX_MU_TYPE_TXDB:
+		imx_mu_xcr_rmw(priv, IMX_MU_xCR_GIRn(cp->idx), 0);
+		schedule_work(&cp->work);
+		break;
+	default:
+		dev_warn_ratelimited(priv->dev, "Send data on wrong channel type: %d\n", cp->type);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int imx_mu_startup(struct mbox_chan *chan)
+{
+	struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox);
+	struct imx_mu_con_priv *cp = chan->con_priv;
+	int ret;
+
+	if (cp->type == IMX_MU_TYPE_TXDB) {
+		/* Tx doorbell don't have ACK support */
+		INIT_WORK(&cp->work, imx_mu_txdb_work);
+		return 0;
+	}
+
+	cp->irq_desc = kasprintf(GFP_KERNEL, "imx_mu_chan[%i-%i]", cp->type,
+				 cp->idx);
+	if (!cp->irq_desc)
+		return -ENOMEM;
+
+	ret = request_irq(priv->irq, imx_mu_isr, IRQF_SHARED, cp->irq_desc,
+			  chan);
+	if (ret) {
+		dev_err(priv->dev,
+			"Unable to acquire IRQ %d\n", priv->irq);
+		return ret;
+	}
+
+	switch (cp->type) {
+		break;
+	case IMX_MU_TYPE_RX:
+		imx_mu_xcr_rmw(priv, IMX_MU_xCR_RIEn(cp->idx), 0);
+		break;
+	case IMX_MU_TYPE_RXDB:
+		imx_mu_xcr_rmw(priv, IMX_MU_xCR_GIEn(cp->idx), 0);
+		break;
+	default:
+		break;
+	}
+
+	return 0;
+}
+
+static void imx_mu_shutdown(struct mbox_chan *chan)
+{
+	struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox);
+	struct imx_mu_con_priv *cp = chan->con_priv;
+
+	if (IMX_MU_TYPE_TXDB == cp->type)
+		flush_work(&cp->work);
+
+	imx_mu_xcr_rmw(priv, 0,
+		   IMX_MU_xCR_TIEn(cp->idx) | IMX_MU_xCR_RIEn(cp->idx));
+
+	free_irq(priv->irq, chan);
+	kfree(cp->irq_desc);
+}
+
+static const struct mbox_chan_ops imx_mu_ops = {
+	.send_data = imx_mu_send_data,
+	.startup = imx_mu_startup,
+	.shutdown = imx_mu_shutdown,
+};
+
+static struct mbox_chan * imx_mu_xlate(struct mbox_controller *mbox,
+				       const struct of_phandle_args *sp)
+{
+	u32 type, idx, chan;
+
+	if (sp->args_count != 2) {
+		dev_err(mbox->dev, "Invalid argument count %d\n", sp->args_count);
+		return ERR_PTR(-EINVAL);
+	}
+
+	type = sp->args[0]; /* channel type */
+	idx = sp->args[1]; /* index */
+	chan = type * 4 + idx;
+
+	if (chan >= mbox->num_chans) {
+		dev_err(mbox->dev, "Not supported channel number: %d. (type: %d, idx: %d)\n", chan, type, idx);
+		return ERR_PTR(-EINVAL);
+	}
+
+	return &mbox->chans[chan];
+}
+
+static void imx_mu_init_generic(struct imx_mu_priv *priv)
+{
+	if (priv->side_b)
+		return;
+
+	/* Set default MU configuration */
+	imx_mu_write(priv, 0, IMX_MU_xCR);
+}
+
+static int imx_mu_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct resource *iomem;
+	struct imx_mu_priv *priv;
+	unsigned int i;
+	int ret;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->dev = dev;
+
+	iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	priv->base = devm_ioremap_resource(&pdev->dev, iomem);
+	if (IS_ERR(priv->base))
+		return PTR_ERR(priv->base);
+
+	priv->irq = platform_get_irq(pdev, 0);
+	if (priv->irq < 0)
+		return priv->irq;
+
+	priv->clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(priv->clk)) {
+		if (PTR_ERR(priv->clk) != -ENOENT)
+			return PTR_ERR(priv->clk);
+
+		priv->clk = NULL;
+	}
+
+	ret = clk_prepare_enable(priv->clk);
+	if (ret) {
+		dev_err(dev, "Failed to enable clock\n");
+		return ret;
+	}
+
+	for (i = 0; i < IMX_MU_CHANS; i++) {
+		struct imx_mu_con_priv *cp = &priv->con_priv[i];
+
+		cp->idx = i % 4;
+		cp->type = (i - cp->idx) >> 2;
+		cp->chan = &priv->mbox_chans[i];
+		priv->mbox_chans[i].con_priv = cp;
+	}
+
+	priv->side_b = of_property_read_bool(np, "fsl,mu-side-b");
+
+	spin_lock_init(&priv->xcr_lock);
+
+	priv->mbox.dev = dev;
+	priv->mbox.ops = &imx_mu_ops;
+	priv->mbox.chans = priv->mbox_chans;
+	priv->mbox.num_chans = IMX_MU_CHANS;
+	priv->mbox.of_xlate = imx_mu_xlate;
+	priv->mbox.txdone_irq = true;
+
+	platform_set_drvdata(pdev, priv);
+
+	imx_mu_init_generic(priv);
+
+	return mbox_controller_register(&priv->mbox);
+}
+
+static int imx_mu_remove(struct platform_device *pdev)
+{
+	struct imx_mu_priv *priv = platform_get_drvdata(pdev);
+
+	mbox_controller_unregister(&priv->mbox);
+	clk_disable_unprepare(priv->clk);
+
+	return 0;
+}
+
+static const struct of_device_id imx_mu_dt_ids[] = {
+	{ .compatible = "fsl,imx6sx-mu" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, imx_mu_dt_ids);
+
+static struct platform_driver imx_mu_driver = {
+	.probe		= imx_mu_probe,
+	.remove		= imx_mu_remove,
+	.driver = {
+		.name	= "imx_mu",
+		.of_match_table = imx_mu_dt_ids,
+	},
+};
+module_platform_driver(imx_mu_driver);
+
+MODULE_AUTHOR("Oleksij Rempel <o.rempel@pengutronix.de>");
+MODULE_DESCRIPTION("Message Unit driver for i.MX");
+MODULE_LICENSE("GPL v2");