Re: [PATCHv7 2/5] mailbox: Introduce framework for mailbox

2014-06-22 Thread Lubomir Rintel
On Thu, 2014-06-12 at 22:31 +0530, 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
...
> +struct mbox_chan *mbox_request_channel(const struct mbox_client *cl)
> +{
...
> + chan->cl = cl;

drivers/mailbox/mailbox.c: In function ‘mbox_request_channel’:
drivers/mailbox/mailbox.c:361:11: warning: assignment discards ‘const’
qualifier from pointer target type [enabled by default]
  chan->cl = cl;


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCHv7 2/5] mailbox: Introduce framework for mailbox

2014-06-22 Thread Lubomir Rintel
On Thu, 2014-06-12 at 22:31 +0530, 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
...
 +struct mbox_chan *mbox_request_channel(const struct mbox_client *cl)
 +{
...
 + chan-cl = cl;

drivers/mailbox/mailbox.c: In function ‘mbox_request_channel’:
drivers/mailbox/mailbox.c:361:11: warning: assignment discards ‘const’
qualifier from pointer target type [enabled by default]
  chan-cl = cl;


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCHv7 2/5] mailbox: Introduce framework for mailbox

2014-06-20 Thread Lubomir Rintel
On Thu, 2014-06-12 at 22:31 +0530, 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 

> +/**
> + * mbox_chan_received_data - A way for controller driver to push data
> + *   received from remote to the upper layer.
> + * @chan: Pointer to the mailbox channel on which RX happened.
> + * @data: Client specific message typecasted as void *

It's "mssg", not "data".

> +static struct mbox_chan *
> +of_mbox_index_xlate(struct mbox_controller *mbox,
> + const struct of_phandle_args *sp)

The line break here is inconsistent with how the rest of the file is
formatted.

> +/**
> + * struct mbox_controller - Controller of a class of communication chans
> + * @dev: Device backing this controller
> + * @controller_name: Literal name of the controller.
> + * @ops: Operators that work on each communication chan
> + * @chans:   Null terminated array of chans.

This needs to be updated for current API. It's neither not NULL
terminated nor and array and num_chans documentation is missing.

> + * @txdone_irq:  Indicates if the controller can report to API 
> when
> + *   the last transmitted data was read by the remote.
> + *   Eg, if it has some TX ACK irq.
> + * @txdone_poll: If the controller can read but not report the TX
> + *   done. Ex, some register shows the TX status but
> + *   no interrupt rises. Ignored if 'txdone_irq' is set.
> + * @txpoll_period:   If 'txdone_poll' is in effect, the API polls for
> + *   last TX's status after these many millisecs
> + */
> +struct mbox_controller {
> + struct device *dev;
> + struct mbox_chan_ops *ops;
> + struct mbox_chan *chans;
> + int num_chans;
> + bool txdone_irq;
> + bool txdone_poll;
> + unsigned txpoll_period;
> + struct mbox_chan *(*of_xlate)(struct mbox_controller *mbox,
> + const struct of_phandle_args *sp);
> + /*
> +  * If the controller supports only TXDONE_BY_POLL,
> +  * this timer polls all the links for txdone.
> +  */
> + struct timer_list poll;
> + unsigned period;
> + /* Hook to add to the global controller list */
> + struct list_head node;
> +};

Thank you,
Lubo

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCHv7 2/5] mailbox: Introduce framework for mailbox

2014-06-20 Thread Lubomir Rintel
On Thu, 2014-06-12 at 22:31 +0530, 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.si...@linaro.org

 +/**
 + * mbox_chan_received_data - A way for controller driver to push data
 + *   received from remote to the upper layer.
 + * @chan: Pointer to the mailbox channel on which RX happened.
 + * @data: Client specific message typecasted as void *

It's mssg, not data.

 +static struct mbox_chan *
 +of_mbox_index_xlate(struct mbox_controller *mbox,
 + const struct of_phandle_args *sp)

The line break here is inconsistent with how the rest of the file is
formatted.

 +/**
 + * struct mbox_controller - Controller of a class of communication chans
 + * @dev: Device backing this controller
 + * @controller_name: Literal name of the controller.
 + * @ops: Operators that work on each communication chan
 + * @chans:   Null terminated array of chans.

This needs to be updated for current API. It's neither not NULL
terminated nor and array and num_chans documentation is missing.

 + * @txdone_irq:  Indicates if the controller can report to API 
 when
 + *   the last transmitted data was read by the remote.
 + *   Eg, if it has some TX ACK irq.
 + * @txdone_poll: If the controller can read but not report the TX
 + *   done. Ex, some register shows the TX status but
 + *   no interrupt rises. Ignored if 'txdone_irq' is set.
 + * @txpoll_period:   If 'txdone_poll' is in effect, the API polls for
 + *   last TX's status after these many millisecs
 + */
 +struct mbox_controller {
 + struct device *dev;
 + struct mbox_chan_ops *ops;
 + struct mbox_chan *chans;
 + int num_chans;
 + bool txdone_irq;
 + bool txdone_poll;
 + unsigned txpoll_period;
 + struct mbox_chan *(*of_xlate)(struct mbox_controller *mbox,
 + const struct of_phandle_args *sp);
 + /*
 +  * If the controller supports only TXDONE_BY_POLL,
 +  * this timer polls all the links for txdone.
 +  */
 + struct timer_list poll;
 + unsigned period;
 + /* Hook to add to the global controller list */
 + struct list_head node;
 +};

Thank you,
Lubo

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCHv7 2/5] mailbox: Introduce framework for mailbox

2014-06-19 Thread Matt Porter
On Fri, Jun 20, 2014 at 01:59:30AM +0530, Jassi Brar wrote:
> On 20 June 2014 00:33, Matt Porter  wrote:
> > On Thu, Jun 19, 2014 at 07:17:11PM +0100, Sudeep Holla wrote:
> >
> >> >+ * After startup and before shutdown any data received on the chan
> >> >+ * is passed on to the API via atomic mbox_chan_received_data().
> >> >+ * The controller should ACK the RX only after this call returns.
> >>
> >> Does this mean we can't support asynchronous messages from the remote.
> >> One possible scenario I can think is if the remote system power controller
> >> has feature to configure the bounds for thermal sensors and it can send
> >> async interrupt when the bounds are crossed. We can't just block one 
> >> channel
> >> for this always. Again this might have been discussed before and you might 
> >> have
> >> solution, I could not gather it with my brief look at older discussions.
> >
> > The way I see it we are simply putting the burden on the client to
> > implement very little in the rx_callback. In my case, we will have a
> > single client which is the IPC layer. The controller driver will notify
> > the IPC client layer which will do as little as possible in the
> > rx_callback before returning. We'll handle asynchronous dispatch of
> > events within our IPC layer to the real client drivers rather than in
> > the controller driver.
> >
> Yes. So do I.
> 
> >> >+/**
> >> >+ * mbox_client_peek_data - A way for client driver to pull data
> >> >+ * received from remote by the controller.
> >> >+ * @chan: Mailbox channel assigned to this client.
> >> >+ *
> >> >+ * A poke to controller driver for any received data.
> >> >+ * The data is actually passed onto client via the
> >> >+ * mbox_chan_received_data()
> >> >+ * The call can be made from atomic context, so the controller's
> >> >+ * implementation of peek_data() must not sleep.
> >> >+ *
> >> >+ * Return: True, if controller has, and is going to push after this,
> >> >+ *  some data.
> >> >+ * False, if controller doesn't have any data to be read.
> >> >+ */
> >> >+bool mbox_client_peek_data(struct mbox_chan *chan)
> >> >+{
> >> >+   if (chan->mbox->ops->peek_data)
> >> >+   return chan->mbox->ops->peek_data(chan);
> >> >+
> >> >+   return false;
> >> >+}
> >> >+EXPORT_SYMBOL_GPL(mbox_client_peek_data);
> >>
> >> I am unable to understand how this API will be used. IIUC when the 
> >> controller
> >> receives any data from remote, it calls mbox_chan_received_data to push 
> >> data to
> >> client.
> >
> > Good question.
> >
> > That function is a no-op if your client chooses not to populate
> > rx_callback. It's not explicitly stated, but the implementation is a
> > no-op if rx_callback is NULL so rx_callback seems to be intended as an
> > optional field in the client data.
> >
> > I'm also not clear of the scenario where this could be used. I
> > originally thought .peek_data() was an alternative to the callback for
> > polling purposes except it clearly states it needs the callback to carry
> > the data.
> >
> > I probably missed earlier discussion that explains this.
> >
> peek_data is just a trigger for controller to flush out any buffered
> RX via mbox_chan_received_data() to upper layer. Intended usecase is
> irq-mitigation for QMTM driver, as Arnd pointed out a few months ago.

Ok, that makes much more sense now.

Thanks,
Matt
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCHv7 2/5] mailbox: Introduce framework for mailbox

2014-06-19 Thread Jassi Brar
On 20 June 2014 00:33, Matt Porter  wrote:
> On Thu, Jun 19, 2014 at 07:17:11PM +0100, Sudeep Holla wrote:
>
>> >+ * After startup and before shutdown any data received on the chan
>> >+ * is passed on to the API via atomic mbox_chan_received_data().
>> >+ * The controller should ACK the RX only after this call returns.
>>
>> Does this mean we can't support asynchronous messages from the remote.
>> One possible scenario I can think is if the remote system power controller
>> has feature to configure the bounds for thermal sensors and it can send
>> async interrupt when the bounds are crossed. We can't just block one channel
>> for this always. Again this might have been discussed before and you might 
>> have
>> solution, I could not gather it with my brief look at older discussions.
>
> The way I see it we are simply putting the burden on the client to
> implement very little in the rx_callback. In my case, we will have a
> single client which is the IPC layer. The controller driver will notify
> the IPC client layer which will do as little as possible in the
> rx_callback before returning. We'll handle asynchronous dispatch of
> events within our IPC layer to the real client drivers rather than in
> the controller driver.
>
Yes. So do I.

>> >+/**
>> >+ * mbox_client_peek_data - A way for client driver to pull data
>> >+ * received from remote by the controller.
>> >+ * @chan: Mailbox channel assigned to this client.
>> >+ *
>> >+ * A poke to controller driver for any received data.
>> >+ * The data is actually passed onto client via the
>> >+ * mbox_chan_received_data()
>> >+ * The call can be made from atomic context, so the controller's
>> >+ * implementation of peek_data() must not sleep.
>> >+ *
>> >+ * Return: True, if controller has, and is going to push after this,
>> >+ *  some data.
>> >+ * False, if controller doesn't have any data to be read.
>> >+ */
>> >+bool mbox_client_peek_data(struct mbox_chan *chan)
>> >+{
>> >+   if (chan->mbox->ops->peek_data)
>> >+   return chan->mbox->ops->peek_data(chan);
>> >+
>> >+   return false;
>> >+}
>> >+EXPORT_SYMBOL_GPL(mbox_client_peek_data);
>>
>> I am unable to understand how this API will be used. IIUC when the controller
>> receives any data from remote, it calls mbox_chan_received_data to push data 
>> to
>> client.
>
> Good question.
>
> That function is a no-op if your client chooses not to populate
> rx_callback. It's not explicitly stated, but the implementation is a
> no-op if rx_callback is NULL so rx_callback seems to be intended as an
> optional field in the client data.
>
> I'm also not clear of the scenario where this could be used. I
> originally thought .peek_data() was an alternative to the callback for
> polling purposes except it clearly states it needs the callback to carry
> the data.
>
> I probably missed earlier discussion that explains this.
>
peek_data is just a trigger for controller to flush out any buffered
RX via mbox_chan_received_data() to upper layer. Intended usecase is
irq-mitigation for QMTM driver, as Arnd pointed out a few months ago.

Thanks
-Jassi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCHv7 2/5] mailbox: Introduce framework for mailbox

2014-06-19 Thread Sudeep Holla

Hi Jassi,

I started using this from v5 and tried briefly to follow previous versions
and discussion. Please forgive me if I ask questions that are already answered.

On 12/06/14 18:01, 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 
---
  drivers/mailbox/Makefile   |   4 +
  drivers/mailbox/mailbox.c  | 487 +
  include/linux/mailbox_client.h |  45 
  include/linux/mailbox_controller.h | 121 +
  4 files changed, 657 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/Makefile b/drivers/mailbox/Makefile
index e0facb3..2fa343a 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -1,3 +1,7 @@
+# Generic MAILBOX API
+
+obj-$(CONFIG_MAILBOX)  += mailbox.o
+
  obj-$(CONFIG_PL320_MBOX)   += pl320-ipc.o

  obj-$(CONFIG_OMAP_MBOX)+= omap-mailbox.o
diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
new file mode 100644
index 000..336fa10
--- /dev/null
+++ b/drivers/mailbox/mailbox.c
@@ -0,0 +1,487 @@
+/*
+ * Mailbox: Common code for Mailbox controllers and users
+ *
+ * Copyright (C) 2014 Linaro Ltd.
+ * Author: Jassi Brar 
+ *
+ * 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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#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(>lock, flags);
+
+   /* See if there is any space left */
+   if (chan->msg_count == MBOX_TX_QUEUE_LEN) {
+   spin_unlock_irqrestore(>lock, flags);
+   return -ENOMEM;
+   }
+


Any particular reason why the standard list implementation can't be used
instead of this. It eliminates limitation of MBOX_TX_QUEUE_LEN and would
remove msg_free/count


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

Re: [PATCHv7 2/5] mailbox: Introduce framework for mailbox

2014-06-19 Thread Sudeep Holla

Hi Jassi,

On 19/06/14 03:55, Jassi Brar wrote:

On 18 June 2014 22:33, Kevin Hilman  wrote:


[...]


That's great.

I sure would like to see some more Reviewed-by tags from those folks to
confirm that those starting to use it think it's on the right track.


The upstreaming attempts have been going on for months now, and via
non-public interactions with developers I understand it last worked
before the revision mandating DT support and ipc->mailbox symbol
renaming. So basic working should still remain the same.
Suman(TI), Loic(ST), Girish(Samsung), Ashwin (PCC+ACPI) guys,
any word for v7?



I have been using v6 on ARM64 development platform and in the process of moving
to v7. I will respond with my comments/queries separately.

Regards,
Sudeep


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCHv7 2/5] mailbox: Introduce framework for mailbox

2014-06-19 Thread Ashwin Chaugule
Hello,

On 18 June 2014 22:55, Jassi Brar  wrote:
>>
>> I sure would like to see some more Reviewed-by tags from those folks to
>> confirm that those starting to use it think it's on the right track.
>>
> The upstreaming attempts have been going on for months now, and via
> non-public interactions with developers I understand it last worked
> before the revision mandating DT support and ipc->mailbox symbol
> renaming. So basic working should still remain the same.
>Suman(TI), Loic(ST), Girish(Samsung), Ashwin (PCC+ACPI) guys,
> any word for v7?


V7 looks fine overall but it needs some minor changes for ACPI.
Something along the lines of what I submitted previously.[1]

[1] - http://www.spinics.net/lists/linux-acpi/msg51156.html

Cheers,
Ashwin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCHv7 2/5] mailbox: Introduce framework for mailbox

2014-06-19 Thread Ashwin Chaugule
Hello,

On 18 June 2014 22:55, Jassi Brar jaswinder.si...@linaro.org wrote:

 I sure would like to see some more Reviewed-by tags from those folks to
 confirm that those starting to use it think it's on the right track.

 The upstreaming attempts have been going on for months now, and via
 non-public interactions with developers I understand it last worked
 before the revision mandating DT support and ipc-mailbox symbol
 renaming. So basic working should still remain the same.
Suman(TI), Loic(ST), Girish(Samsung), Ashwin (PCC+ACPI) guys,
 any word for v7?


V7 looks fine overall but it needs some minor changes for ACPI.
Something along the lines of what I submitted previously.[1]

[1] - http://www.spinics.net/lists/linux-acpi/msg51156.html

Cheers,
Ashwin
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCHv7 2/5] mailbox: Introduce framework for mailbox

2014-06-19 Thread Sudeep Holla

Hi Jassi,

On 19/06/14 03:55, Jassi Brar wrote:

On 18 June 2014 22:33, Kevin Hilman khil...@linaro.org wrote:


[...]


That's great.

I sure would like to see some more Reviewed-by tags from those folks to
confirm that those starting to use it think it's on the right track.


The upstreaming attempts have been going on for months now, and via
non-public interactions with developers I understand it last worked
before the revision mandating DT support and ipc-mailbox symbol
renaming. So basic working should still remain the same.
Suman(TI), Loic(ST), Girish(Samsung), Ashwin (PCC+ACPI) guys,
any word for v7?



I have been using v6 on ARM64 development platform and in the process of moving
to v7. I will respond with my comments/queries separately.

Regards,
Sudeep


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCHv7 2/5] mailbox: Introduce framework for mailbox

2014-06-19 Thread Sudeep Holla

Hi Jassi,

I started using this from v5 and tried briefly to follow previous versions
and discussion. Please forgive me if I ask questions that are already answered.

On 12/06/14 18:01, 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.si...@linaro.org
---
  drivers/mailbox/Makefile   |   4 +
  drivers/mailbox/mailbox.c  | 487 +
  include/linux/mailbox_client.h |  45 
  include/linux/mailbox_controller.h | 121 +
  4 files changed, 657 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/Makefile b/drivers/mailbox/Makefile
index e0facb3..2fa343a 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -1,3 +1,7 @@
+# Generic MAILBOX API
+
+obj-$(CONFIG_MAILBOX)  += mailbox.o
+
  obj-$(CONFIG_PL320_MBOX)   += pl320-ipc.o

  obj-$(CONFIG_OMAP_MBOX)+= omap-mailbox.o
diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
new file mode 100644
index 000..336fa10
--- /dev/null
+++ b/drivers/mailbox/mailbox.c
@@ -0,0 +1,487 @@
+/*
+ * Mailbox: Common code for Mailbox controllers and users
+ *
+ * Copyright (C) 2014 Linaro Ltd.
+ * Author: Jassi Brar jassisinghb...@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;
+   }
+


Any particular reason why the standard list implementation can't be used
instead of this. It eliminates limitation of MBOX_TX_QUEUE_LEN and would
remove msg_free/count


+   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);
+   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 (chan-cl-tx_block)
+   complete(chan-tx_complete);
+   else if (mssg  chan-cl-tx_done)
+   chan-cl-tx_done(chan-cl, mssg, r);
+}
+
+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) {
+   

Re: [PATCHv7 2/5] mailbox: Introduce framework for mailbox

2014-06-19 Thread Jassi Brar
On 20 June 2014 00:33, Matt Porter mpor...@linaro.org wrote:
 On Thu, Jun 19, 2014 at 07:17:11PM +0100, Sudeep Holla wrote:

 + * After startup and before shutdown any data received on the chan
 + * is passed on to the API via atomic mbox_chan_received_data().
 + * The controller should ACK the RX only after this call returns.

 Does this mean we can't support asynchronous messages from the remote.
 One possible scenario I can think is if the remote system power controller
 has feature to configure the bounds for thermal sensors and it can send
 async interrupt when the bounds are crossed. We can't just block one channel
 for this always. Again this might have been discussed before and you might 
 have
 solution, I could not gather it with my brief look at older discussions.

 The way I see it we are simply putting the burden on the client to
 implement very little in the rx_callback. In my case, we will have a
 single client which is the IPC layer. The controller driver will notify
 the IPC client layer which will do as little as possible in the
 rx_callback before returning. We'll handle asynchronous dispatch of
 events within our IPC layer to the real client drivers rather than in
 the controller driver.

Yes. So do I.

 +/**
 + * mbox_client_peek_data - A way for client driver to pull data
 + * received from remote by the controller.
 + * @chan: Mailbox channel assigned to this client.
 + *
 + * A poke to controller driver for any received data.
 + * The data is actually passed onto client via the
 + * mbox_chan_received_data()
 + * The call can be made from atomic context, so the controller's
 + * implementation of peek_data() must not sleep.
 + *
 + * Return: True, if controller has, and is going to push after this,
 + *  some data.
 + * False, if controller doesn't have any data to be read.
 + */
 +bool mbox_client_peek_data(struct mbox_chan *chan)
 +{
 +   if (chan-mbox-ops-peek_data)
 +   return chan-mbox-ops-peek_data(chan);
 +
 +   return false;
 +}
 +EXPORT_SYMBOL_GPL(mbox_client_peek_data);

 I am unable to understand how this API will be used. IIUC when the controller
 receives any data from remote, it calls mbox_chan_received_data to push data 
 to
 client.

 Good question.

 That function is a no-op if your client chooses not to populate
 rx_callback. It's not explicitly stated, but the implementation is a
 no-op if rx_callback is NULL so rx_callback seems to be intended as an
 optional field in the client data.

 I'm also not clear of the scenario where this could be used. I
 originally thought .peek_data() was an alternative to the callback for
 polling purposes except it clearly states it needs the callback to carry
 the data.

 I probably missed earlier discussion that explains this.

peek_data is just a trigger for controller to flush out any buffered
RX via mbox_chan_received_data() to upper layer. Intended usecase is
irq-mitigation for QMTM driver, as Arnd pointed out a few months ago.

Thanks
-Jassi
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCHv7 2/5] mailbox: Introduce framework for mailbox

2014-06-19 Thread Matt Porter
On Fri, Jun 20, 2014 at 01:59:30AM +0530, Jassi Brar wrote:
 On 20 June 2014 00:33, Matt Porter mpor...@linaro.org wrote:
  On Thu, Jun 19, 2014 at 07:17:11PM +0100, Sudeep Holla wrote:
 
  + * After startup and before shutdown any data received on the chan
  + * is passed on to the API via atomic mbox_chan_received_data().
  + * The controller should ACK the RX only after this call returns.
 
  Does this mean we can't support asynchronous messages from the remote.
  One possible scenario I can think is if the remote system power controller
  has feature to configure the bounds for thermal sensors and it can send
  async interrupt when the bounds are crossed. We can't just block one 
  channel
  for this always. Again this might have been discussed before and you might 
  have
  solution, I could not gather it with my brief look at older discussions.
 
  The way I see it we are simply putting the burden on the client to
  implement very little in the rx_callback. In my case, we will have a
  single client which is the IPC layer. The controller driver will notify
  the IPC client layer which will do as little as possible in the
  rx_callback before returning. We'll handle asynchronous dispatch of
  events within our IPC layer to the real client drivers rather than in
  the controller driver.
 
 Yes. So do I.
 
  +/**
  + * mbox_client_peek_data - A way for client driver to pull data
  + * received from remote by the controller.
  + * @chan: Mailbox channel assigned to this client.
  + *
  + * A poke to controller driver for any received data.
  + * The data is actually passed onto client via the
  + * mbox_chan_received_data()
  + * The call can be made from atomic context, so the controller's
  + * implementation of peek_data() must not sleep.
  + *
  + * Return: True, if controller has, and is going to push after this,
  + *  some data.
  + * False, if controller doesn't have any data to be read.
  + */
  +bool mbox_client_peek_data(struct mbox_chan *chan)
  +{
  +   if (chan-mbox-ops-peek_data)
  +   return chan-mbox-ops-peek_data(chan);
  +
  +   return false;
  +}
  +EXPORT_SYMBOL_GPL(mbox_client_peek_data);
 
  I am unable to understand how this API will be used. IIUC when the 
  controller
  receives any data from remote, it calls mbox_chan_received_data to push 
  data to
  client.
 
  Good question.
 
  That function is a no-op if your client chooses not to populate
  rx_callback. It's not explicitly stated, but the implementation is a
  no-op if rx_callback is NULL so rx_callback seems to be intended as an
  optional field in the client data.
 
  I'm also not clear of the scenario where this could be used. I
  originally thought .peek_data() was an alternative to the callback for
  polling purposes except it clearly states it needs the callback to carry
  the data.
 
  I probably missed earlier discussion that explains this.
 
 peek_data is just a trigger for controller to flush out any buffered
 RX via mbox_chan_received_data() to upper layer. Intended usecase is
 irq-mitigation for QMTM driver, as Arnd pointed out a few months ago.

Ok, that makes much more sense now.

Thanks,
Matt
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCHv7 2/5] mailbox: Introduce framework for mailbox

2014-06-18 Thread Jassi Brar
On 18 June 2014 22:33, Kevin Hilman  wrote:
> Jassi Brar  writes:
>
>> On 18 June 2014 05:57, Kevin Hilman  wrote:
>>> Jassi Brar  writes:
>>>
 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 
>>>
>>> This series is shaping up nicely.  The one thing I think it would
>>> benefit from, being a new common framework is something under
>>> Documentation giving a brief overview, but more importantly some
>>> example code snippets of a mailbox client using the API, and maybe an
>>> example usage of the controller API as well.
>>>
>>> Not only will that guide developers who want to use/implement this API
>>> on their platforms, it will also aid reviewers.
>>>
>> I have been trying to get it upstream for quite some time now because
>> my platform depends upon it. I am planning to submit my platform
>> support which should have a client and controller side of the mailbox
>> API.
>
> Having a reference implementation is great, but I don't think that
> removes the need for a bit of Documentation when introducing a new
> framework.
>
> It's pretty common to see new IPC mechanisms posted and being able to
> point somone to this framework and something under Documentation/* would
> be a great help in getting more users of the framework.
>
Of course. I didn't mean I won't add Documentation.

>> Though I am told the API (until v4 at least) supported usecases for 5
>> different platforms.
>
> That's great.
>
> I sure would like to see some more Reviewed-by tags from those folks to
> confirm that those starting to use it think it's on the right track.
>
The upstreaming attempts have been going on for months now, and via
non-public interactions with developers I understand it last worked
before the revision mandating DT support and ipc->mailbox symbol
renaming. So basic working should still remain the same.
   Suman(TI), Loic(ST), Girish(Samsung), Ashwin (PCC+ACPI) guys,
any word for v7?

LFTan(Intel) and Craig(Broadcom) seem unresponsive now, unfortunately :(

Thanks
-Jassi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCHv7 2/5] mailbox: Introduce framework for mailbox

2014-06-18 Thread Kevin Hilman
Jassi Brar  writes:

> On 18 June 2014 05:57, Kevin Hilman  wrote:
>> Jassi Brar  writes:
>>
>>> 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 
>>
>> This series is shaping up nicely.  The one thing I think it would
>> benefit from, being a new common framework is something under
>> Documentation giving a brief overview, but more importantly some
>> example code snippets of a mailbox client using the API, and maybe an
>> example usage of the controller API as well.
>>
>> Not only will that guide developers who want to use/implement this API
>> on their platforms, it will also aid reviewers.
>>
> I have been trying to get it upstream for quite some time now because
> my platform depends upon it. I am planning to submit my platform
> support which should have a client and controller side of the mailbox
> API. 

Having a reference implementation is great, but I don't think that
removes the need for a bit of Documentation when introducing a new
framework.  

It's pretty common to see new IPC mechanisms posted and being able to
point somone to this framework and something under Documentation/* would
be a great help in getting more users of the framework.

> Though I am told the API (until v4 at least) supported usecases for 5
> different platforms.

That's great.

I sure would like to see some more Reviewed-by tags from those folks to
confirm that those starting to use it think it's on the right track.

Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCHv7 2/5] mailbox: Introduce framework for mailbox

2014-06-18 Thread Jassi Brar
On 18 June 2014 05:57, Kevin Hilman  wrote:
> Jassi Brar  writes:
>
>> 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 
>
> This series is shaping up nicely.  The one thing I think it would
> benefit from, being a new common framework is something under
> Documentation giving a brief overview, but more importantly some
> example code snippets of a mailbox client using the API, and maybe an
> example usage of the controller API as well.
>
> Not only will that guide developers who want to use/implement this API
> on their platforms, it will also aid reviewers.
>
I have been trying to get it upstream for quite some time now because
my platform depends upon it. I am planning to submit my platform
support which should have a client and controller side of the mailbox
API. Though I am told the API (until v4 at least) supported usecases
for 5 different platforms.

Thanks,
Jassi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCHv7 2/5] mailbox: Introduce framework for mailbox

2014-06-18 Thread Jassi Brar
On 18 June 2014 05:57, Kevin Hilman khil...@linaro.org wrote:
 Jassi Brar jaswinder.si...@linaro.org writes:

 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.si...@linaro.org

 This series is shaping up nicely.  The one thing I think it would
 benefit from, being a new common framework is something under
 Documentation giving a brief overview, but more importantly some
 example code snippets of a mailbox client using the API, and maybe an
 example usage of the controller API as well.

 Not only will that guide developers who want to use/implement this API
 on their platforms, it will also aid reviewers.

I have been trying to get it upstream for quite some time now because
my platform depends upon it. I am planning to submit my platform
support which should have a client and controller side of the mailbox
API. Though I am told the API (until v4 at least) supported usecases
for 5 different platforms.

Thanks,
Jassi
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCHv7 2/5] mailbox: Introduce framework for mailbox

2014-06-18 Thread Kevin Hilman
Jassi Brar jaswinder.si...@linaro.org writes:

 On 18 June 2014 05:57, Kevin Hilman khil...@linaro.org wrote:
 Jassi Brar jaswinder.si...@linaro.org writes:

 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.si...@linaro.org

 This series is shaping up nicely.  The one thing I think it would
 benefit from, being a new common framework is something under
 Documentation giving a brief overview, but more importantly some
 example code snippets of a mailbox client using the API, and maybe an
 example usage of the controller API as well.

 Not only will that guide developers who want to use/implement this API
 on their platforms, it will also aid reviewers.

 I have been trying to get it upstream for quite some time now because
 my platform depends upon it. I am planning to submit my platform
 support which should have a client and controller side of the mailbox
 API. 

Having a reference implementation is great, but I don't think that
removes the need for a bit of Documentation when introducing a new
framework.  

It's pretty common to see new IPC mechanisms posted and being able to
point somone to this framework and something under Documentation/* would
be a great help in getting more users of the framework.

 Though I am told the API (until v4 at least) supported usecases for 5
 different platforms.

That's great.

I sure would like to see some more Reviewed-by tags from those folks to
confirm that those starting to use it think it's on the right track.

Kevin
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCHv7 2/5] mailbox: Introduce framework for mailbox

2014-06-18 Thread Jassi Brar
On 18 June 2014 22:33, Kevin Hilman khil...@linaro.org wrote:
 Jassi Brar jaswinder.si...@linaro.org writes:

 On 18 June 2014 05:57, Kevin Hilman khil...@linaro.org wrote:
 Jassi Brar jaswinder.si...@linaro.org writes:

 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.si...@linaro.org

 This series is shaping up nicely.  The one thing I think it would
 benefit from, being a new common framework is something under
 Documentation giving a brief overview, but more importantly some
 example code snippets of a mailbox client using the API, and maybe an
 example usage of the controller API as well.

 Not only will that guide developers who want to use/implement this API
 on their platforms, it will also aid reviewers.

 I have been trying to get it upstream for quite some time now because
 my platform depends upon it. I am planning to submit my platform
 support which should have a client and controller side of the mailbox
 API.

 Having a reference implementation is great, but I don't think that
 removes the need for a bit of Documentation when introducing a new
 framework.

 It's pretty common to see new IPC mechanisms posted and being able to
 point somone to this framework and something under Documentation/* would
 be a great help in getting more users of the framework.

Of course. I didn't mean I won't add Documentation.

 Though I am told the API (until v4 at least) supported usecases for 5
 different platforms.

 That's great.

 I sure would like to see some more Reviewed-by tags from those folks to
 confirm that those starting to use it think it's on the right track.

The upstreaming attempts have been going on for months now, and via
non-public interactions with developers I understand it last worked
before the revision mandating DT support and ipc-mailbox symbol
renaming. So basic working should still remain the same.
   Suman(TI), Loic(ST), Girish(Samsung), Ashwin (PCC+ACPI) guys,
any word for v7?

LFTan(Intel) and Craig(Broadcom) seem unresponsive now, unfortunately :(

Thanks
-Jassi
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCHv7 2/5] mailbox: Introduce framework for mailbox

2014-06-17 Thread Kevin Hilman
Jassi Brar  writes:

> 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 

This series is shaping up nicely.  The one thing I think it would
benefit from, being a new common framework is something under
Documentation giving a brief overview, but more importantly some 
example code snippets of a mailbox client using the API, and maybe an
example usage of the controller API as well.

Not only will that guide developers who want to use/implement this API
on their platforms, it will also aid reviewers.

Thanks,

Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCHv7 2/5] mailbox: Introduce framework for mailbox

2014-06-17 Thread Kevin Hilman
Jassi Brar jaswinder.si...@linaro.org writes:

 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.si...@linaro.org

This series is shaping up nicely.  The one thing I think it would
benefit from, being a new common framework is something under
Documentation giving a brief overview, but more importantly some 
example code snippets of a mailbox client using the API, and maybe an
example usage of the controller API as well.

Not only will that guide developers who want to use/implement this API
on their platforms, it will also aid reviewers.

Thanks,

Kevin
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCHv7 2/5] mailbox: Introduce framework for mailbox

2014-06-13 Thread Mark Brown
On Thu, Jun 12, 2014 at 10:31:19PM +0530, Jassi Brar wrote:

A couple of tiny nits, I'll send followup patches for these.

> +bool mbox_client_peek_data(struct mbox_chan *chan)
> +{
> + if (chan->mbox->ops->peek_data)
> + return chan->mbox->ops->peek_data(chan);
> +
> + return false;
> +}
> +EXPORT_SYMBOL_GPL(mbox_client_peek_data);

This isn't declared in the header to allow users to use it.

> +struct mbox_chan *mbox_request_channel(const struct mbox_client *cl)

> + spin_lock_irqsave(>lock, flags);
> + chan->msg_free = 0;
> + chan->msg_count = 0;
> + chan->active_req = NULL;
> + chan->cl = cl;

chan->cl is non-const but cl is const so this assignment is invalid.


signature.asc
Description: Digital signature


Re: [PATCHv7 2/5] mailbox: Introduce framework for mailbox

2014-06-13 Thread Mark Brown
On Thu, Jun 12, 2014 at 10:31:19PM +0530, Jassi Brar wrote:

A couple of tiny nits, I'll send followup patches for these.

 +bool mbox_client_peek_data(struct mbox_chan *chan)
 +{
 + if (chan-mbox-ops-peek_data)
 + return chan-mbox-ops-peek_data(chan);
 +
 + return false;
 +}
 +EXPORT_SYMBOL_GPL(mbox_client_peek_data);

This isn't declared in the header to allow users to use it.

 +struct mbox_chan *mbox_request_channel(const struct mbox_client *cl)

 + spin_lock_irqsave(chan-lock, flags);
 + chan-msg_free = 0;
 + chan-msg_count = 0;
 + chan-active_req = NULL;
 + chan-cl = cl;

chan-cl is non-const but cl is const so this assignment is invalid.


signature.asc
Description: Digital signature


[PATCHv7 2/5] mailbox: Introduce framework for mailbox

2014-06-12 Thread Jassi Brar
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 
---
 drivers/mailbox/Makefile   |   4 +
 drivers/mailbox/mailbox.c  | 487 +
 include/linux/mailbox_client.h |  45 
 include/linux/mailbox_controller.h | 121 +
 4 files changed, 657 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/Makefile b/drivers/mailbox/Makefile
index e0facb3..2fa343a 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -1,3 +1,7 @@
+# Generic MAILBOX API
+
+obj-$(CONFIG_MAILBOX)  += mailbox.o
+
 obj-$(CONFIG_PL320_MBOX)   += pl320-ipc.o
 
 obj-$(CONFIG_OMAP_MBOX)+= omap-mailbox.o
diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
new file mode 100644
index 000..336fa10
--- /dev/null
+++ b/drivers/mailbox/mailbox.c
@@ -0,0 +1,487 @@
+/*
+ * Mailbox: Common code for Mailbox controllers and users
+ *
+ * Copyright (C) 2014 Linaro Ltd.
+ * Author: Jassi Brar 
+ *
+ * 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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#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(>lock, flags);
+
+   /* See if there is any space left */
+   if (chan->msg_count == MBOX_TX_QUEUE_LEN) {
+   spin_unlock_irqrestore(>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(>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(>lock, flags);
+
+   if (!chan->msg_count || chan->active_req) {
+   spin_unlock_irqrestore(>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);
+   if (!err) {
+   chan->active_req = data;
+   chan->msg_count--;
+   }
+
+   spin_unlock_irqrestore(>lock, flags);
+}
+
+static void tx_tick(struct mbox_chan *chan, int r)
+{
+   unsigned long flags;
+   void *mssg;
+
+   spin_lock_irqsave(>lock, flags);
+   mssg = chan->active_req;
+   chan->active_req = NULL;
+   spin_unlock_irqrestore(>lock, flags);
+
+   /* Submit next message */
+   _msg_submit(chan);
+
+   /* Notify the client */
+   if (chan->cl->tx_block)
+   complete(>tx_complete);
+   else if (mssg && chan->cl->tx_done)
+   chan->cl->tx_done(chan->cl, mssg, r);
+}
+
+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 = >chans[i];
+
+   if (chan->active_req && chan->cl) {
+   resched = true;
+   txdone = chan->mbox->ops->last_tx_done(chan);
+   if (txdone)
+   tx_tick(chan, 0);
+   }
+   }
+
+   if (resched)
+   mod_timer(>poll,
+   jiffies + msecs_to_jiffies(mbox->period));
+}
+
+/**
+ * mbox_chan_received_data - A way for controller driver to push data
+ * received from remote to the upper layer.
+ * @chan: Pointer to the mailbox channel on which RX happened.
+ * @data: Client specific message 

[PATCHv7 2/5] mailbox: Introduce framework for mailbox

2014-06-12 Thread Jassi Brar
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.si...@linaro.org
---
 drivers/mailbox/Makefile   |   4 +
 drivers/mailbox/mailbox.c  | 487 +
 include/linux/mailbox_client.h |  45 
 include/linux/mailbox_controller.h | 121 +
 4 files changed, 657 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/Makefile b/drivers/mailbox/Makefile
index e0facb3..2fa343a 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -1,3 +1,7 @@
+# Generic MAILBOX API
+
+obj-$(CONFIG_MAILBOX)  += mailbox.o
+
 obj-$(CONFIG_PL320_MBOX)   += pl320-ipc.o
 
 obj-$(CONFIG_OMAP_MBOX)+= omap-mailbox.o
diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
new file mode 100644
index 000..336fa10
--- /dev/null
+++ b/drivers/mailbox/mailbox.c
@@ -0,0 +1,487 @@
+/*
+ * Mailbox: Common code for Mailbox controllers and users
+ *
+ * Copyright (C) 2014 Linaro Ltd.
+ * Author: Jassi Brar jassisinghb...@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);
+   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 (chan-cl-tx_block)
+   complete(chan-tx_complete);
+   else if (mssg  chan-cl-tx_done)
+   chan-cl-tx_done(chan-cl, mssg, r);
+}
+
+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);
+   }
+   }
+
+   if (resched)
+   mod_timer(mbox-poll,
+   jiffies + msecs_to_jiffies(mbox-period));
+}
+
+/**
+ * mbox_chan_received_data - A way for