On 4 November 2014 at 04:35, Suman Anna <s-anna@xxxxxx> wrote: > The OMAP mailbox driver and its existing clients (remoteproc > for OMAP4+) are adapted to use the generic mailbox framework. > > The main changes for the adaptation are: > - The tasklet used for Tx is replaced with the state machine from > the generic mailbox framework. The workqueue used for processing > the received messages stays intact for minimizing the effects on > the OMAP mailbox clients. > - The existing exported client API, omap_mbox_get, omap_mbox_put and > omap_mbox_send_msg are deleted, as the framework provides equivalent > functionality. A OMAP-specific omap_mbox_request_channel is added > though to support non-DT way of requesting mailboxes. > - The OMAP mailbox driver is integrated with the mailbox framework > through the proper implementations of mbox_chan_ops, except for > .last_tx_done and .peek_data. The OMAP mailbox driver does not need > these ops, as it is completely interrupt driven. > - The OMAP mailbox driver uses a custom of_xlate controller ops that > allows phandles for the pargs specifier instead of indexing to avoid > any channel registration order dependencies. > - The new framework does not support multiple clients operating on a > single channel, so the reference counting logic is simplified. > - The remoteproc driver (current client) is adapted to use the new API. > The notifier callbacks used within this client is replaced with the > regular callbacks from the newer framework. > - The exported OMAP mailbox API are limited to omap_mbox_save_ctx, > omap_mbox_restore_ctx, omap_mbox_enable_irq & omap_mbox_disable_irq, > with the signature modified to take in the new mbox_chan handle instead > of the OMAP specific omap_mbox handle. The first 2 will be removed when > the OMAP mailbox driver is adapted to runtime_pm. The other exported > API omap_mbox_request_channel will be removed once existing legacy > users are converted to DT. > > Cc: Jassi Brar <jassisinghbrar@xxxxxxxxx> > Cc: Ohad Ben-Cohen <ohad@xxxxxxxxxx> > Signed-off-by: Suman Anna <s-anna@xxxxxx> > --- > v3->v4: No code changes, switched the example to use the DSP node instead of > WkupM3 in the bindings document & minor commit description changes. Other than > that, this is a repost of the driver adaptation patch [1] from the OMAP Mailbox > framework adaptation series [2]. This patch is intended for the 3.19 merge window, > all the dependent patches in [2] are merged as of 3.18-rc2. The DTS patch in [2] > will be posted separately. > > [1] http://marc.info/?l=linux-omap&m=141038453917790&w=2 > [2] http://marc.info/?l=linux-omap&m=141038447817775&w=2 > > .../devicetree/bindings/mailbox/omap-mailbox.txt | 23 ++ > drivers/mailbox/omap-mailbox.c | 346 ++++++++++++--------- > drivers/remoteproc/omap_remoteproc.c | 51 +-- > include/linux/omap-mailbox.h | 16 +- > 4 files changed, 256 insertions(+), 180 deletions(-) > > diff --git a/Documentation/devicetree/bindings/mailbox/omap-mailbox.txt b/Documentation/devicetree/bindings/mailbox/omap-mailbox.txt > index 48edc4b..d1a0433 100644 > --- a/Documentation/devicetree/bindings/mailbox/omap-mailbox.txt > +++ b/Documentation/devicetree/bindings/mailbox/omap-mailbox.txt > @@ -43,6 +43,9 @@ Required properties: > device. The format is dependent on which interrupt > controller the OMAP device uses > - ti,hwmods: Name of the hwmod associated with the mailbox > +- #mbox-cells: Common mailbox binding property to identify the number > + of cells required for the mailbox specifier. Should be > + 1 > - ti,mbox-num-users: Number of targets (processor devices) that the mailbox > device can interrupt > - ti,mbox-num-fifos: Number of h/w fifo queues within the mailbox IP block > @@ -72,6 +75,18 @@ data that represent the following: > Cell #3 (usr_id) - mailbox user id for identifying the interrupt line > associated with generating a tx/rx fifo interrupt. > > +Mailbox Users: > +============== > +A device needing to communicate with a target processor device should specify > +them using the common mailbox binding properties, "mboxes" and the optional > +"mbox-names" (please see Documentation/devicetree/bindings/mailbox/mailbox.txt > +for details). Each value of the mboxes property should contain a phandle to the > +mailbox controller device node and an args specifier that will be the phandle to > +the intended sub-mailbox child node to be used for communication. The equivalent > +"mbox-names" property value can be used to give a name to the communication channel > +to be used by the client user. > + > + > Example: > -------- > > @@ -81,6 +96,7 @@ mailbox: mailbox@4a0f4000 { > reg = <0x4a0f4000 0x200>; > interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>; > ti,hwmods = "mailbox"; > + #mbox-cells = <1>; > ti,mbox-num-users = <3>; > ti,mbox-num-fifos = <8>; > mbox_ipu: mbox_ipu { > @@ -93,12 +109,19 @@ mailbox: mailbox@4a0f4000 { > }; > }; > > +dsp { > + ... > + mboxes = <&mailbox &mbox_dsp>; > + ... > +}; > + > /* AM33xx */ > mailbox: mailbox@480C8000 { > compatible = "ti,omap4-mailbox"; > reg = <0x480C8000 0x200>; > interrupts = <77>; > ti,hwmods = "mailbox"; > + #mbox-cells = <1>; > ti,mbox-num-users = <4>; > ti,mbox-num-fifos = <8>; > mbox_wkupm3: wkup_m3 { > diff --git a/drivers/mailbox/omap-mailbox.c b/drivers/mailbox/omap-mailbox.c > index bcc7ee1..66b83ca 100644 > --- a/drivers/mailbox/omap-mailbox.c > +++ b/drivers/mailbox/omap-mailbox.c > @@ -29,13 +29,14 @@ > #include <linux/slab.h> > #include <linux/kfifo.h> > #include <linux/err.h> > -#include <linux/notifier.h> > #include <linux/module.h> > #include <linux/of_device.h> > #include <linux/platform_device.h> > #include <linux/pm_runtime.h> > #include <linux/platform_data/mailbox-omap.h> > #include <linux/omap-mailbox.h> > +#include <linux/mailbox_controller.h> > +#include <linux/mailbox_client.h> > > #define MAILBOX_REVISION 0x000 > #define MAILBOX_MESSAGE(m) (0x040 + 4 * (m)) > @@ -80,7 +81,6 @@ struct omap_mbox_queue { > spinlock_t lock; > struct kfifo fifo; > struct work_struct work; > - struct tasklet_struct tasklet; > struct omap_mbox *mbox; > bool full; > }; > @@ -92,6 +92,7 @@ struct omap_mbox_device { > u32 num_users; > u32 num_fifos; > struct omap_mbox **mboxes; > + struct mbox_controller controller; > struct list_head elem; > }; > > @@ -110,15 +111,14 @@ struct omap_mbox_fifo_info { > struct omap_mbox { > const char *name; > int irq; > - struct omap_mbox_queue *txq, *rxq; > + struct omap_mbox_queue *rxq; > struct device *dev; > struct omap_mbox_device *parent; > struct omap_mbox_fifo tx_fifo; > struct omap_mbox_fifo rx_fifo; > u32 ctx[OMAP4_MBOX_NR_REGS]; > u32 intr_type; > - int use_count; > - struct blocking_notifier_head notifier; > + struct mbox_chan *chan; > }; > > /* global variables for the mailbox devices */ > @@ -129,6 +129,14 @@ static unsigned int mbox_kfifo_size = CONFIG_OMAP_MBOX_KFIFO_SIZE; > module_param(mbox_kfifo_size, uint, S_IRUGO); > MODULE_PARM_DESC(mbox_kfifo_size, "Size of omap's mailbox kfifo (bytes)"); > > +static struct omap_mbox *mbox_chan_to_omap_mbox(struct mbox_chan *chan) > +{ > + if (!chan || !chan->con_priv) > + return NULL; > + > + return (struct omap_mbox *)chan->con_priv; > +} > + > static inline > unsigned int mbox_read_reg(struct omap_mbox_device *mdev, size_t ofs) > { > @@ -194,41 +202,14 @@ static int is_mbox_irq(struct omap_mbox *mbox, omap_mbox_irq_t irq) > return (int)(enable & status & bit); > } > > -/* > - * message sender > - */ > -int omap_mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg) > -{ > - struct omap_mbox_queue *mq = mbox->txq; > - int ret = 0, len; > - > - spin_lock_bh(&mq->lock); > - > - if (kfifo_avail(&mq->fifo) < sizeof(msg)) { > - ret = -ENOMEM; > - goto out; > - } > - > - if (kfifo_is_empty(&mq->fifo) && !mbox_fifo_full(mbox)) { > - mbox_fifo_write(mbox, msg); > - goto out; > - } > - > - len = kfifo_in(&mq->fifo, (unsigned char *)&msg, sizeof(msg)); > - WARN_ON(len != sizeof(msg)); > - > - tasklet_schedule(&mbox->txq->tasklet); > - > -out: > - spin_unlock_bh(&mq->lock); > - return ret; > -} > -EXPORT_SYMBOL(omap_mbox_msg_send); > - > -void omap_mbox_save_ctx(struct omap_mbox *mbox) > +void omap_mbox_save_ctx(struct mbox_chan *chan) > { > int i; > int nr_regs; > + struct omap_mbox *mbox = mbox_chan_to_omap_mbox(chan); > + > + if (WARN_ON(!mbox)) > + return; > > if (mbox->intr_type) > nr_regs = OMAP4_MBOX_NR_REGS; > @@ -243,10 +224,14 @@ void omap_mbox_save_ctx(struct omap_mbox *mbox) > } > EXPORT_SYMBOL(omap_mbox_save_ctx); > > -void omap_mbox_restore_ctx(struct omap_mbox *mbox) > +void omap_mbox_restore_ctx(struct mbox_chan *chan) > { > int i; > int nr_regs; > + struct omap_mbox *mbox = mbox_chan_to_omap_mbox(chan); > + > + if (WARN_ON(!mbox)) > + return; > > if (mbox->intr_type) > nr_regs = OMAP4_MBOX_NR_REGS; > @@ -254,14 +239,13 @@ void omap_mbox_restore_ctx(struct omap_mbox *mbox) > nr_regs = MBOX_NR_REGS; > for (i = 0; i < nr_regs; i++) { > mbox_write_reg(mbox->parent, mbox->ctx[i], i * sizeof(u32)); > - > dev_dbg(mbox->dev, "%s: [%02x] %08x\n", __func__, > i, mbox->ctx[i]); > } > } > EXPORT_SYMBOL(omap_mbox_restore_ctx); > > -void omap_mbox_enable_irq(struct omap_mbox *mbox, omap_mbox_irq_t irq) > +static void _omap_mbox_enable_irq(struct omap_mbox *mbox, omap_mbox_irq_t irq) > { > u32 l; > struct omap_mbox_fifo *fifo = (irq == IRQ_TX) ? > @@ -273,9 +257,8 @@ void omap_mbox_enable_irq(struct omap_mbox *mbox, omap_mbox_irq_t irq) > l |= bit; > mbox_write_reg(mbox->parent, l, irqenable); > } > -EXPORT_SYMBOL(omap_mbox_enable_irq); > > -void omap_mbox_disable_irq(struct omap_mbox *mbox, omap_mbox_irq_t irq) > +static void _omap_mbox_disable_irq(struct omap_mbox *mbox, omap_mbox_irq_t irq) > { > struct omap_mbox_fifo *fifo = (irq == IRQ_TX) ? > &mbox->tx_fifo : &mbox->rx_fifo; > @@ -291,28 +274,28 @@ void omap_mbox_disable_irq(struct omap_mbox *mbox, omap_mbox_irq_t irq) > > mbox_write_reg(mbox->parent, bit, irqdisable); > } > -EXPORT_SYMBOL(omap_mbox_disable_irq); > > -static void mbox_tx_tasklet(unsigned long tx_data) > +void omap_mbox_enable_irq(struct mbox_chan *chan, omap_mbox_irq_t irq) > { > - struct omap_mbox *mbox = (struct omap_mbox *)tx_data; > - struct omap_mbox_queue *mq = mbox->txq; > - mbox_msg_t msg; > - int ret; > + struct omap_mbox *mbox = mbox_chan_to_omap_mbox(chan); > > - while (kfifo_len(&mq->fifo)) { > - if (mbox_fifo_full(mbox)) { > - omap_mbox_enable_irq(mbox, IRQ_TX); > - break; > - } > + if (WARN_ON(!mbox)) > + return; > > - ret = kfifo_out(&mq->fifo, (unsigned char *)&msg, > - sizeof(msg)); > - WARN_ON(ret != sizeof(msg)); > + _omap_mbox_enable_irq(mbox, irq); > +} > +EXPORT_SYMBOL(omap_mbox_enable_irq); > > - mbox_fifo_write(mbox, msg); > - } > +void omap_mbox_disable_irq(struct mbox_chan *chan, omap_mbox_irq_t irq) > +{ > + struct omap_mbox *mbox = mbox_chan_to_omap_mbox(chan); > + > + if (WARN_ON(!mbox)) > + return; > + > + _omap_mbox_disable_irq(mbox, irq); > } > +EXPORT_SYMBOL(omap_mbox_disable_irq); > > /* > * Message receiver(workqueue) > @@ -328,12 +311,11 @@ static void mbox_rx_work(struct work_struct *work) > len = kfifo_out(&mq->fifo, (unsigned char *)&msg, sizeof(msg)); > WARN_ON(len != sizeof(msg)); > > - blocking_notifier_call_chain(&mq->mbox->notifier, len, > - (void *)msg); > + mbox_chan_received_data(mq->mbox->chan, (void *)msg); > spin_lock_irq(&mq->lock); > if (mq->full) { > mq->full = false; > - omap_mbox_enable_irq(mq->mbox, IRQ_RX); > + _omap_mbox_enable_irq(mq->mbox, IRQ_RX); > } > spin_unlock_irq(&mq->lock); > } > @@ -344,9 +326,9 @@ static void mbox_rx_work(struct work_struct *work) > */ > static void __mbox_tx_interrupt(struct omap_mbox *mbox) > { > - omap_mbox_disable_irq(mbox, IRQ_TX); > + _omap_mbox_disable_irq(mbox, IRQ_TX); > ack_mbox_irq(mbox, IRQ_TX); > - tasklet_schedule(&mbox->txq->tasklet); > + mbox_chan_txdone(mbox->chan, 0); > } > > static void __mbox_rx_interrupt(struct omap_mbox *mbox) > @@ -357,7 +339,7 @@ static void __mbox_rx_interrupt(struct omap_mbox *mbox) > > while (!mbox_fifo_empty(mbox)) { > if (unlikely(kfifo_avail(&mq->fifo) < sizeof(msg))) { > - omap_mbox_disable_irq(mbox, IRQ_RX); > + _omap_mbox_disable_irq(mbox, IRQ_RX); > mq->full = true; > goto nomem; > } > @@ -388,11 +370,13 @@ static irqreturn_t mbox_interrupt(int irq, void *p) > } > > static struct omap_mbox_queue *mbox_queue_alloc(struct omap_mbox *mbox, > - void (*work) (struct work_struct *), > - void (*tasklet)(unsigned long)) > + void (*work)(struct work_struct *)) > { > struct omap_mbox_queue *mq; > > + if (!work) > + return NULL; > + > mq = kzalloc(sizeof(struct omap_mbox_queue), GFP_KERNEL); > if (!mq) > return NULL; > @@ -402,12 +386,9 @@ static struct omap_mbox_queue *mbox_queue_alloc(struct omap_mbox *mbox, > if (kfifo_alloc(&mq->fifo, mbox_kfifo_size, GFP_KERNEL)) > goto error; > > - if (work) > - INIT_WORK(&mq->work, work); > - > - if (tasklet) > - tasklet_init(&mq->tasklet, tasklet, (unsigned long)mbox); > + INIT_WORK(&mq->work, work); > return mq; > + > error: > kfree(mq); > return NULL; > @@ -423,71 +404,35 @@ static int omap_mbox_startup(struct omap_mbox *mbox) > { > int ret = 0; > struct omap_mbox_queue *mq; > - struct omap_mbox_device *mdev = mbox->parent; > > - mutex_lock(&mdev->cfg_lock); > - ret = pm_runtime_get_sync(mdev->dev); > - if (unlikely(ret < 0)) > - goto fail_startup; > - > - if (!mbox->use_count++) { > - mq = mbox_queue_alloc(mbox, NULL, mbox_tx_tasklet); > - if (!mq) { > - ret = -ENOMEM; > - goto fail_alloc_txq; > - } > - mbox->txq = mq; > + mq = mbox_queue_alloc(mbox, mbox_rx_work); > + if (!mq) > + return -ENOMEM; > + mbox->rxq = mq; > + mq->mbox = mbox; > + > + ret = request_irq(mbox->irq, mbox_interrupt, IRQF_SHARED, > + mbox->name, mbox); > + if (unlikely(ret)) { > + pr_err("failed to register mailbox interrupt:%d\n", ret); > + goto fail_request_irq; > + } > > - mq = mbox_queue_alloc(mbox, mbox_rx_work, NULL); > - if (!mq) { > - ret = -ENOMEM; > - goto fail_alloc_rxq; > - } > - mbox->rxq = mq; > - mq->mbox = mbox; > - ret = request_irq(mbox->irq, mbox_interrupt, IRQF_SHARED, > - mbox->name, mbox); > - if (unlikely(ret)) { > - pr_err("failed to register mailbox interrupt:%d\n", > - ret); > - goto fail_request_irq; > - } > + _omap_mbox_enable_irq(mbox, IRQ_RX); > > - omap_mbox_enable_irq(mbox, IRQ_RX); > - } > - mutex_unlock(&mdev->cfg_lock); > return 0; > > fail_request_irq: > mbox_queue_free(mbox->rxq); > -fail_alloc_rxq: > - mbox_queue_free(mbox->txq); > -fail_alloc_txq: > - pm_runtime_put_sync(mdev->dev); > - mbox->use_count--; > -fail_startup: > - mutex_unlock(&mdev->cfg_lock); > return ret; > } > > static void omap_mbox_fini(struct omap_mbox *mbox) > { > - struct omap_mbox_device *mdev = mbox->parent; > - > - mutex_lock(&mdev->cfg_lock); > - > - if (!--mbox->use_count) { > - omap_mbox_disable_irq(mbox, IRQ_RX); > - free_irq(mbox->irq, mbox); > - tasklet_kill(&mbox->txq->tasklet); > - flush_work(&mbox->rxq->work); > - mbox_queue_free(mbox->txq); > - mbox_queue_free(mbox->rxq); > - } > - > - pm_runtime_put_sync(mdev->dev); > - > - mutex_unlock(&mdev->cfg_lock); > + _omap_mbox_disable_irq(mbox, IRQ_RX); > + free_irq(mbox->irq, mbox); > + flush_work(&mbox->rxq->work); > + mbox_queue_free(mbox->rxq); > } > > static struct omap_mbox *omap_mbox_device_find(struct omap_mbox_device *mdev, > @@ -509,42 +454,55 @@ static struct omap_mbox *omap_mbox_device_find(struct omap_mbox_device *mdev, > return mbox; > } > > -struct omap_mbox *omap_mbox_get(const char *name, struct notifier_block *nb) > +struct mbox_chan *omap_mbox_request_channel(struct mbox_client *cl, > + const char *chan_name) > { > + struct device *dev = cl->dev; > struct omap_mbox *mbox = NULL; > struct omap_mbox_device *mdev; > + struct mbox_chan *chan; > + unsigned long flags; > int ret; > > + if (!dev) > + return ERR_PTR(-ENODEV); > + > + if (dev->of_node) { > + pr_err("%s: please use mbox_request_channel(), this API is supported only for OMAP non-DT usage\n", > + __func__); > + return ERR_PTR(-ENODEV); > + } > + > mutex_lock(&omap_mbox_devices_lock); > list_for_each_entry(mdev, &omap_mbox_devices, elem) { > - mbox = omap_mbox_device_find(mdev, name); > + mbox = omap_mbox_device_find(mdev, chan_name); > if (mbox) > break; > } > mutex_unlock(&omap_mbox_devices_lock); > > - if (!mbox) > + if (!mbox || !mbox->chan) > return ERR_PTR(-ENOENT); > > - if (nb) > - blocking_notifier_chain_register(&mbox->notifier, nb); > + chan = mbox->chan; > + spin_lock_irqsave(&chan->lock, flags); > + chan->msg_free = 0; > + chan->msg_count = 0; > + chan->active_req = NULL; > + chan->cl = cl; > + init_completion(&chan->tx_complete); > + spin_unlock_irqrestore(&chan->lock, flags); > > - ret = omap_mbox_startup(mbox); > + ret = chan->mbox->ops->startup(chan); > if (ret) { > - blocking_notifier_chain_unregister(&mbox->notifier, nb); > - return ERR_PTR(-ENODEV); > + pr_err("Unable to startup the chan (%d)\n", ret); > + mbox_free_channel(chan); > + chan = ERR_PTR(ret); > } > > - return mbox; > -} > -EXPORT_SYMBOL(omap_mbox_get); > - > -void omap_mbox_put(struct omap_mbox *mbox, struct notifier_block *nb) > -{ > - blocking_notifier_chain_unregister(&mbox->notifier, nb); > - omap_mbox_fini(mbox); > + return chan; > } > -EXPORT_SYMBOL(omap_mbox_put); > +EXPORT_SYMBOL(omap_mbox_request_channel); > Why not mbox_request_channel()? And what about other 4 exports - omap_mbox_{save_ctx, restore_ctx, enable_irq & disable_irq} ? -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
