On Mon, Dec 22, 2014 at 3:14 PM, Ley Foon Tan <lftan@xxxxxxxxxx> wrote: ... > diff --git a/drivers/mailbox/mailbox-altera.c b/drivers/mailbox/mailbox-altera.c > new file mode 100644 > index 0000000..8019795 > --- /dev/null > +++ b/drivers/mailbox/mailbox-altera.c > @@ -0,0 +1,385 @@ > +/* > + * Copyright Altera Corporation (C) 2013-2014. All rights reserved > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms and conditions of the GNU General Public License, > + * version 2, as published by the Free Software Foundation. > + * > + * This program is distributed in the hope it will be useful, but WITHOUT > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for > + * more details. > + * > + * You should have received a copy of the GNU General Public License along with > + * this program. If not, see <http://www.gnu.org/licenses/>. > + */ > + > +#include <linux/device.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.h> > +#include <linux/platform_device.h> > + > +#define DRIVER_NAME "altera-mailbox" > + > +#define MAILBOX_CMD_REG 0x00 > +#define MAILBOX_PTR_REG 0x04 > +#define MAILBOX_STS_REG 0x08 > +#define MAILBOX_INTMASK_REG 0x0C > + > +#define INT_PENDING_MSK 0x1 > +#define INT_SPACE_MSK 0x2 > + > +#define STS_PENDING_MSK 0x1 > +#define STS_FULL_MSK 0x2 > +#define STS_FULL_OFT 0x1 > + > +#define MBOX_PENDING(status) (((status) & STS_PENDING_MSK)) > +#define MBOX_FULL(status) (((status) & STS_FULL_MSK) >> STS_FULL_OFT) > + > +enum altera_mbox_msg { > + MBOX_CMD = 0, > + MBOX_PTR, > +}; > + > +#define MBOX_POLLING_MS 1 /* polling interval 1ms */ > + The intention is too aggressive - though poll won't be before next jiffy (>1ms). And if you are not expecting a reply, it should be even bigger. Say, 50ms default and 5ms when you expect RX? > +struct altera_mbox { > + bool is_sender; /* 1-sender, 0-receiver */ > + bool intr_mode; > + int irq; > + void __iomem *mbox_base; > + struct device *dev; > + struct mbox_controller controller; > + > + /* If the controller supports only RX polling mode */ > + struct timer_list rxpoll_timer; > +}; > + > +static struct altera_mbox *mbox_chan_to_altera_mbox(struct mbox_chan *chan) > +{ > + if (!chan || !chan->con_priv) > + return NULL; > + > + return (struct altera_mbox *)chan->con_priv; > +} > + > +static inline int altera_mbox_full(struct altera_mbox *mbox) > +{ > + u32 status; > + > + status = readl(mbox->mbox_base + MAILBOX_STS_REG); > + return MBOX_FULL(status); > +} > s/readl/readl_relaxed and s/writel/writel_relaxed everywhere > + > +static inline int altera_mbox_pending(struct altera_mbox *mbox) > +{ > + u32 status; > + > + status = readl(mbox->mbox_base + MAILBOX_STS_REG); > + return MBOX_PENDING(status); > +} > + > +static void altera_mbox_rx_intmask(struct altera_mbox *mbox, bool enable) > +{ > + u32 mask; > + > + mask = readl(mbox->mbox_base + MAILBOX_INTMASK_REG); > + if (enable) > + mask |= INT_PENDING_MSK; > + else > + mask &= ~INT_PENDING_MSK; > + writel(mask, mbox->mbox_base + MAILBOX_INTMASK_REG); > +} > + > +static void altera_mbox_tx_intmask(struct altera_mbox *mbox, bool enable) > +{ > + u32 mask; > + > + mask = readl(mbox->mbox_base + MAILBOX_INTMASK_REG); > + if (enable) > + mask |= INT_SPACE_MSK; > + else > + mask &= ~INT_SPACE_MSK; > + writel(mask, mbox->mbox_base + MAILBOX_INTMASK_REG); > +} > + > +static bool altera_mbox_is_sender(struct altera_mbox *mbox) > +{ > + u32 reg; > + /* Write a magic number to PTR register and read back this register. > + * This register is read-write if it is a sender. > + */ > + #define MBOX_MAGIC 0xA5A5AA55 > + writel(MBOX_MAGIC, mbox->mbox_base + MAILBOX_PTR_REG); > + reg = readl(mbox->mbox_base + MAILBOX_PTR_REG); > + if (reg == MBOX_MAGIC) { > + /* Clear to 0 */ > + writel(0, mbox->mbox_base + MAILBOX_PTR_REG); > + return true; > + } > + return false; > +} > + > +static void altera_mbox_rx_data(struct mbox_chan *chan) > +{ > + struct altera_mbox *mbox = mbox_chan_to_altera_mbox(chan); > + u32 data[2]; > + > + if (altera_mbox_pending(mbox)) { > + data[MBOX_PTR] = readl(mbox->mbox_base + MAILBOX_PTR_REG); > + data[MBOX_CMD] = readl(mbox->mbox_base + MAILBOX_CMD_REG); > + mbox_chan_received_data(chan, (void *)data); > + } > +} > + > +static void altera_mbox_poll_rx(unsigned long data) > +{ > + struct mbox_chan *chan = (struct mbox_chan *)data; > + struct altera_mbox *mbox = mbox_chan_to_altera_mbox(chan); > + > + altera_mbox_rx_data(chan); > + > + mod_timer(&mbox->rxpoll_timer, > + jiffies + msecs_to_jiffies(MBOX_POLLING_MS)); > +} > + > +static irqreturn_t altera_mbox_tx_interrupt(int irq, void *p) > +{ > + struct mbox_chan *chan = (struct mbox_chan *)p; > + struct altera_mbox *mbox = mbox_chan_to_altera_mbox(chan); > + > + altera_mbox_tx_intmask(mbox, false); > + mbox_chan_txdone(chan, 0); > + > + return IRQ_HANDLED; > +} > + > +static irqreturn_t altera_mbox_rx_interrupt(int irq, void *p) > +{ > + struct mbox_chan *chan = (struct mbox_chan *)p; > + > + altera_mbox_rx_data(chan); > + return IRQ_HANDLED; > +} > + > +static int altera_mbox_startup_sender(struct mbox_chan *chan) > +{ > + int ret; > + struct altera_mbox *mbox = mbox_chan_to_altera_mbox(chan); > + > + if (mbox->intr_mode) { > + ret = request_irq(mbox->irq, altera_mbox_tx_interrupt, 0, > + DRIVER_NAME, chan); > + if (unlikely(ret)) { > + dev_err(mbox->dev, > + "failed to register mailbox interrupt:%d\n", > + ret); > + return ret; > + } > + } > + > + return 0; > +} > + > +static int altera_mbox_startup_receiver(struct mbox_chan *chan) > +{ > + int ret; > + struct altera_mbox *mbox = mbox_chan_to_altera_mbox(chan); > + > + if (mbox->intr_mode) { > + ret = request_irq(mbox->irq, altera_mbox_rx_interrupt, 0, > + DRIVER_NAME, chan); > + if (unlikely(ret)) { > + dev_err(mbox->dev, > + "failed to register mailbox interrupt:%d\n", > + ret); > + return ret; > Is this really fatal? Maybe continue using the timer scheme below? > + } > + altera_mbox_rx_intmask(mbox, true); > + } else { > + /* Setup polling timer */ > + setup_timer(&mbox->rxpoll_timer, altera_mbox_poll_rx, > + (unsigned long)chan); > + mod_timer(&mbox->rxpoll_timer, > + jiffies + msecs_to_jiffies(MBOX_POLLING_MS)); > + } > + > + return 0; > +} > + Just these minor comments. -Jassi -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
