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 |
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
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)
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!
> -----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
> -----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
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.
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);
> -----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 --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");