In current mailbox, a self implemented message array to be used
as message fifo, I am replacing it with kernel kfifo,
in order to make code cleaner.
Signed-off-by: jqlhn <[email protected]>
---
drivers/mailbox/mailbox.c | 33 ++++++++----------------------
drivers/mailbox/omap-mailbox.c | 3 +--
drivers/mailbox/pcc.c | 3 +--
include/linux/mailbox_controller.h | 10 ++++-----
4 files changed, 14 insertions(+), 35 deletions(-)
diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
index 4229b9b5da98..d738bb472cd0 100644
--- a/drivers/mailbox/mailbox.c
+++ b/drivers/mailbox/mailbox.c
@@ -25,59 +25,43 @@ 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) {
+ if (kfifo_is_full(&chan->msg_fifo)) {
spin_unlock_irqrestore(&chan->lock, flags);
return -ENOBUFS;
}
- 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++;
+ kfifo_put(&chan->msg_fifo, mssg);
spin_unlock_irqrestore(&chan->lock, flags);
- return idx;
+ return 0;
}
static void msg_submit(struct mbox_chan *chan)
{
- unsigned count, idx;
unsigned long flags;
void *data;
int err = -EBUSY;
spin_lock_irqsave(&chan->lock, flags);
- if (!chan->msg_count || chan->active_req)
+ if (!kfifo_peek(&chan->msg_fifo, &data) || chan->active_req)
goto exit;
- 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];
-
if (chan->cl->tx_prepare)
chan->cl->tx_prepare(chan->cl, data);
/* 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--;
+ /* Get msg out of fifo */
+ if (!kfifo_get(&chan->msg_fifo, &data))
+ err = -ENODATA;
}
exit:
spin_unlock_irqrestore(&chan->lock, flags);
@@ -379,8 +363,7 @@ struct mbox_chan *mbox_request_channel(struct mbox_client *cl, int index)
}
spin_lock_irqsave(&chan->lock, flags);
- chan->msg_free = 0;
- chan->msg_count = 0;
+ INIT_KFIFO(chan->msg_fifo);
chan->active_req = NULL;
chan->cl = cl;
init_completion(&chan->tx_complete);
diff --git a/drivers/mailbox/omap-mailbox.c b/drivers/mailbox/omap-mailbox.c
index 098c82d87137..b392f79e77b3 100644
--- a/drivers/mailbox/omap-mailbox.c
+++ b/drivers/mailbox/omap-mailbox.c
@@ -443,8 +443,7 @@ struct mbox_chan *omap_mbox_request_channel(struct mbox_client *cl,
chan = mbox->chan;
spin_lock_irqsave(&chan->lock, flags);
- chan->msg_free = 0;
- chan->msg_count = 0;
+ INIT_KFIFO(chan->msg_fifo);
chan->active_req = NULL;
chan->cl = cl;
init_completion(&chan->tx_complete);
diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
index 3c2bc0ca454c..2359cba8381e 100644
--- a/drivers/mailbox/pcc.c
+++ b/drivers/mailbox/pcc.c
@@ -297,8 +297,7 @@ pcc_mbox_request_channel(struct mbox_client *cl, int subspace_id)
dev = chan->mbox->dev;
spin_lock_irqsave(&chan->lock, flags);
- chan->msg_free = 0;
- chan->msg_count = 0;
+ INIT_KFIFO(chan->msg_fifo);
chan->active_req = NULL;
chan->cl = cl;
init_completion(&chan->tx_complete);
diff --git a/include/linux/mailbox_controller.h b/include/linux/mailbox_controller.h
index 6fee33cb52f5..b3bec4f33b6d 100644
--- a/include/linux/mailbox_controller.h
+++ b/include/linux/mailbox_controller.h
@@ -8,6 +8,7 @@
#include <linux/hrtimer.h>
#include <linux/device.h>
#include <linux/completion.h>
+#include <linux/kfifo.h>
struct mbox_chan;
@@ -100,7 +101,7 @@ struct mbox_controller {
* REVISIT: If too many platforms see the "Try increasing MBOX_TX_QUEUE_LEN"
* print, it needs to be taken from config option or somesuch.
*/
-#define MBOX_TX_QUEUE_LEN 20
+#define MBOX_TX_QUEUE_LEN 32
/**
* struct mbox_chan - s/w representation of a communication chan
@@ -109,9 +110,7 @@ struct mbox_controller {
* @cl: Pointer to the current owner of this channel
* @tx_complete: Transmission completion
* @active_req: Currently active request hook
- * @msg_count: No. of mssg currently queued
- * @msg_free: Index of next available mssg slot
- * @msg_data: Hook for data packet
+ * @msg_fifo: Hook for data packet
* @lock: Serialise access to the channel
* @con_priv: Hook for controller driver to attach private data
*/
@@ -121,8 +120,7 @@ struct mbox_chan {
struct mbox_client *cl;
struct completion tx_complete;
void *active_req;
- unsigned msg_count, msg_free;
- void *msg_data[MBOX_TX_QUEUE_LEN];
+ DECLARE_KFIFO(msg_fifo, void*, MBOX_TX_QUEUE_LEN);
spinlock_t lock; /* Serialise access to the channel */
void *con_priv;
};
--
2.34.1
On Fri, Nov 4, 2022 at 10:36 PM jqlhn <[email protected]> wrote:
>
> In current mailbox, a self implemented message array to be used
> as message fifo, I am replacing it with kernel kfifo,
> in order to make code cleaner.
>
> Signed-off-by: jqlhn <[email protected]>
> ---
> drivers/mailbox/mailbox.c | 33 ++++++++----------------------
> drivers/mailbox/omap-mailbox.c | 3 +--
> drivers/mailbox/pcc.c | 3 +--
> include/linux/mailbox_controller.h | 10 ++++-----
> 4 files changed, 14 insertions(+), 35 deletions(-)
>
The circular buffer was implemented using an array because it is
simple enough and we wanted to keep tight control over the efficiency.
While you do manage to reduce 21 lines from 4 files, I am not sure
that is a reasonable tradeoff between readability and history plus
actual overhead of execution.
thanks.