lkml.org 
[lkml]   [2014]   [Jul]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCHv9 2/4] mailbox: Introduce framework for mailbox


On 22/07/14 19:56, Jassi Brar wrote:
> Introduce common framework for client/protocol drivers and
> controller drivers of Inter-Processor-Communication (IPC).
>
> Client driver developers should have a look at
> include/linux/mailbox_client.h to understand the part of
> the API exposed to client drivers.
> Similarly controller driver developers should have a look
> at include/linux/mailbox_controller.h
>
> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
> ---
> MAINTAINERS | 8 +
> drivers/mailbox/Makefile | 4 +
> drivers/mailbox/mailbox.c | 467 +++++++++++++++++++++++++++++++++++++
> include/linux/mailbox_client.h | 45 ++++
> include/linux/mailbox_controller.h | 131 +++++++++++
> 5 files changed, 655 insertions(+)
> create mode 100644 drivers/mailbox/mailbox.c
> create mode 100644 include/linux/mailbox_client.h
> create mode 100644 include/linux/mailbox_controller.h
>

[...]

> diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
> new file mode 100644
> index 0000000..99c0d23
> --- /dev/null
> +++ b/drivers/mailbox/mailbox.c
> @@ -0,0 +1,467 @@
> +/*
> + * Mailbox: Common code for Mailbox controllers and users
> + *
> + * Copyright (C) 2013-2014 Linaro Ltd.
> + * Author: Jassi Brar <jassisinghbrar@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/interrupt.h>
> +#include <linux/spinlock.h>
> +#include <linux/mutex.h>
> +#include <linux/delay.h>
> +#include <linux/slab.h>
> +#include <linux/err.h>
> +#include <linux/module.h>
> +#include <linux/device.h>
> +#include <linux/mailbox_client.h>
> +#include <linux/mailbox_controller.h>
> +
> +#define TXDONE_BY_IRQ (1 << 0) /* controller has remote RTR irq */
> +#define TXDONE_BY_POLL (1 << 1) /* controller can read status of last TX */
> +#define TXDONE_BY_ACK (1 << 2) /* S/W ACK recevied by Client ticks the TX */
> +
> +static LIST_HEAD(mbox_cons);
> +static DEFINE_MUTEX(con_mutex);
> +
> +static int add_to_rbuf(struct mbox_chan *chan, void *mssg)
> +{
> + int idx;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&chan->lock, flags);
> +
> + /* See if there is any space left */
> + if (chan->msg_count == MBOX_TX_QUEUE_LEN) {
> + spin_unlock_irqrestore(&chan->lock, flags);
> + return -ENOMEM;
> + }
> +
> + idx = chan->msg_free;
> + chan->msg_data[idx] = mssg;
> + chan->msg_count++;
> +
> + if (idx == MBOX_TX_QUEUE_LEN - 1)
> + chan->msg_free = 0;
> + else
> + chan->msg_free++;
> +
> + spin_unlock_irqrestore(&chan->lock, flags);
> +
> + return idx;
> +}
> +
> +static void msg_submit(struct mbox_chan *chan)
> +{
> + unsigned count, idx;
> + unsigned long flags;
> + void *data;
> + int err;
> +
> + spin_lock_irqsave(&chan->lock, flags);
> +
> + if (!chan->msg_count || chan->active_req) {
> + spin_unlock_irqrestore(&chan->lock, flags);
> + return;
> + }
> +
> + count = chan->msg_count;
> + idx = chan->msg_free;
> + if (idx >= count)
> + idx -= count;
> + else
> + idx += MBOX_TX_QUEUE_LEN - count;
> +
> + data = chan->msg_data[idx];
> +
> + /* Try to submit a message to the MBOX controller */
> + err = chan->mbox->ops->send_data(chan, data);

Probably this is discussed already, but again I missed to find it
in archives, so asking here. If the protocol has some payload associated
with the message and controller expects it to be in place before calling
send_data, we essentially end up not using this queue at all by waiting
in the protocol layer(may be in it's own queue)

Instead can we have some kind of chan->cl->prepare_data callback so that
client can prepare payload ? This in turn avoids to implement it's
*own Tx queue* and *reusing the mailbox queue*.

Or am I missing something ?

> + if (!err) {
> + chan->active_req = data;
> + chan->msg_count--;
> + }
> +
> + spin_unlock_irqrestore(&chan->lock, flags);
> +}
> +
> +static void tx_tick(struct mbox_chan *chan, int r)
> +{
> + unsigned long flags;
> + void *mssg;
> +
> + spin_lock_irqsave(&chan->lock, flags);
> + mssg = chan->active_req;
> + chan->active_req = NULL;
> + spin_unlock_irqrestore(&chan->lock, flags);
> +
> + /* Submit next message */
> + msg_submit(chan);
> +
> + /* Notify the client */
> + if (mssg && chan->cl->tx_done)
> + chan->cl->tx_done(chan->cl, mssg, r);
> +
> + if (chan->cl->tx_block)
> + complete(&chan->tx_complete);
> +}
> +
> +static void poll_txdone(unsigned long data)
> +{
> + struct mbox_controller *mbox = (struct mbox_controller *)data;
> + bool txdone, resched = false;
> + int i;
> +
> + for (i = 0; i < mbox->num_chans; i++) {
> + struct mbox_chan *chan = &mbox->chans[i];
> +
> + if (chan->active_req && chan->cl) {
> + resched = true;
> + txdone = chan->mbox->ops->last_tx_done(chan);
> + if (txdone)
> + tx_tick(chan, 0);

What if all the active channels finished Tx(i.e. txdone = 1), we still have
resched = true and add a timer, not really harmful though. But IMO you can
move it to else part instead ?


Regards,
Sudeep




\
 
 \ /
  Last update: 2014-07-31 22:01    [W:0.095 / U:0.448 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site