Hello all,
Core of this series is the last patch removing the message FIFO
from OMAP mailbox. This hurts our real-time performance. It was a
legacy leftover from before the common mailbox framework anyway.
The rest of the patches are cleanups found along the way.
Thanks,
Andrew
Andrew Davis (13):
mailbox: omap: Remove unused omap_mbox_{enable,disable}_irq()
functions
mailbox: omap: Remove unused omap_mbox_request_channel() function
mailbox: omap: Move omap_mbox_irq_t into driver
mailbox: omap: Move fifo size check to point of use
mailbox: omap: Remove unneeded header omap-mailbox.h
mailbox: omap: Remove device class
mailbox: omap: Use devm_pm_runtime_enable() helper
mailbox: omap: Merge mailbox child node setup loops
mailbox: omap: Use function local struct mbox_controller
mailbox: omap: Use mbox_controller channel list directly
mailbox: omap: Remove mbox_chan_to_omap_mbox()
mailbox: omap: Reverse FIFO busy check logic
mailbox: omap: Remove kernel FIFO message queuing
drivers/mailbox/Kconfig | 9 -
drivers/mailbox/omap-mailbox.c | 515 +++++++--------------------------
include/linux/omap-mailbox.h | 13 -
3 files changed, 106 insertions(+), 431 deletions(-)
--
2.39.2
The mbox_kfifo_size can be changed at runtime, the sanity
check on it's value should be done when it is used, not
only once at init time.
Signed-off-by: Andrew Davis <[email protected]>
---
drivers/mailbox/omap-mailbox.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/mailbox/omap-mailbox.c b/drivers/mailbox/omap-mailbox.c
index c083734b6954c..167348fb1b33b 100644
--- a/drivers/mailbox/omap-mailbox.c
+++ b/drivers/mailbox/omap-mailbox.c
@@ -310,6 +310,7 @@ static struct omap_mbox_queue *mbox_queue_alloc(struct omap_mbox *mbox,
void (*work)(struct work_struct *))
{
struct omap_mbox_queue *mq;
+ unsigned int size;
if (!work)
return NULL;
@@ -320,7 +321,10 @@ static struct omap_mbox_queue *mbox_queue_alloc(struct omap_mbox *mbox,
spin_lock_init(&mq->lock);
- if (kfifo_alloc(&mq->fifo, mbox_kfifo_size, GFP_KERNEL))
+ /* kfifo size sanity check: alignment and minimal size */
+ size = ALIGN(mbox_kfifo_size, sizeof(u32));
+ size = max_t(unsigned int, size, sizeof(u32));
+ if (kfifo_alloc(&mq->fifo, size, GFP_KERNEL))
goto error;
INIT_WORK(&mq->work, work);
@@ -838,10 +842,6 @@ static int __init omap_mbox_init(void)
if (err)
return err;
- /* kfifo size sanity check: alignment and minimal size */
- mbox_kfifo_size = ALIGN(mbox_kfifo_size, sizeof(u32));
- mbox_kfifo_size = max_t(unsigned int, mbox_kfifo_size, sizeof(u32));
-
err = platform_driver_register(&omap_mbox_driver);
if (err)
class_unregister(&omap_mbox_class);
--
2.39.2
These function are not used, remove these here.
While here, remove the leading _ from the driver internal functions that
do the same thing as the functions removed.
Signed-off-by: Andrew Davis <[email protected]>
---
drivers/mailbox/omap-mailbox.c | 42 ++++++++--------------------------
include/linux/omap-mailbox.h | 3 ---
2 files changed, 10 insertions(+), 35 deletions(-)
diff --git a/drivers/mailbox/omap-mailbox.c b/drivers/mailbox/omap-mailbox.c
index c961706fe61d5..624a7ccc27285 100644
--- a/drivers/mailbox/omap-mailbox.c
+++ b/drivers/mailbox/omap-mailbox.c
@@ -197,7 +197,7 @@ static int is_mbox_irq(struct omap_mbox *mbox, omap_mbox_irq_t irq)
return (int)(enable & status & bit);
}
-static 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) ?
@@ -210,7 +210,7 @@ static void _omap_mbox_enable_irq(struct omap_mbox *mbox, omap_mbox_irq_t irq)
mbox_write_reg(mbox->parent, l, irqenable);
}
-static 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;
@@ -227,28 +227,6 @@ static void _omap_mbox_disable_irq(struct omap_mbox *mbox, omap_mbox_irq_t irq)
mbox_write_reg(mbox->parent, bit, irqdisable);
}
-void omap_mbox_enable_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_enable_irq(mbox, irq);
-}
-EXPORT_SYMBOL(omap_mbox_enable_irq);
-
-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)
*/
@@ -269,7 +247,7 @@ static void mbox_rx_work(struct work_struct *work)
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);
}
@@ -280,7 +258,7 @@ 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);
mbox_chan_txdone(mbox->chan, 0);
}
@@ -293,7 +271,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;
}
@@ -375,7 +353,7 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
if (mbox->send_no_irq)
mbox->chan->txdone_method = TXDONE_BY_ACK;
- _omap_mbox_enable_irq(mbox, IRQ_RX);
+ omap_mbox_enable_irq(mbox, IRQ_RX);
return 0;
@@ -386,7 +364,7 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
static void omap_mbox_fini(struct omap_mbox *mbox)
{
- _omap_mbox_disable_irq(mbox, IRQ_RX);
+ omap_mbox_disable_irq(mbox, IRQ_RX);
free_irq(mbox->irq, mbox);
flush_work(&mbox->rxq->work);
mbox_queue_free(mbox->rxq);
@@ -533,10 +511,10 @@ static int omap_mbox_chan_send_noirq(struct omap_mbox *mbox, u32 msg)
int ret = -EBUSY;
if (!mbox_fifo_full(mbox)) {
- _omap_mbox_enable_irq(mbox, IRQ_RX);
+ omap_mbox_enable_irq(mbox, IRQ_RX);
mbox_fifo_write(mbox, msg);
ret = 0;
- _omap_mbox_disable_irq(mbox, IRQ_RX);
+ omap_mbox_disable_irq(mbox, IRQ_RX);
/* we must read and ack the interrupt directly from here */
mbox_fifo_read(mbox);
@@ -556,7 +534,7 @@ static int omap_mbox_chan_send(struct omap_mbox *mbox, u32 msg)
}
/* always enable the interrupt */
- _omap_mbox_enable_irq(mbox, IRQ_TX);
+ omap_mbox_enable_irq(mbox, IRQ_TX);
return ret;
}
diff --git a/include/linux/omap-mailbox.h b/include/linux/omap-mailbox.h
index 8aa984ec1f38b..426a80fb32b5c 100644
--- a/include/linux/omap-mailbox.h
+++ b/include/linux/omap-mailbox.h
@@ -20,7 +20,4 @@ struct mbox_client;
struct mbox_chan *omap_mbox_request_channel(struct mbox_client *cl,
const char *chan_name);
-void omap_mbox_enable_irq(struct mbox_chan *chan, omap_mbox_irq_t irq);
-void omap_mbox_disable_irq(struct mbox_chan *chan, omap_mbox_irq_t irq);
-
#endif /* OMAP_MAILBOX_H */
--
2.39.2
This function only checks if mbox_chan *chan is not NULL, but that cannot
be the case and if it was returning NULL which is not later checked
doesn't save us from this. The second check for chan->con_priv is
completely redundant as if it was NULL we would return NULL just the
same. Simply dereference con_priv directly and remove this function.
Signed-off-by: Andrew Davis <[email protected]>
---
drivers/mailbox/omap-mailbox.c | 14 +++-----------
1 file changed, 3 insertions(+), 11 deletions(-)
diff --git a/drivers/mailbox/omap-mailbox.c b/drivers/mailbox/omap-mailbox.c
index 8e42266cb31a5..8e2760d2c5b0c 100644
--- a/drivers/mailbox/omap-mailbox.c
+++ b/drivers/mailbox/omap-mailbox.c
@@ -103,14 +103,6 @@ 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)
{
@@ -357,7 +349,7 @@ static void omap_mbox_fini(struct omap_mbox *mbox)
static int omap_mbox_chan_startup(struct mbox_chan *chan)
{
- struct omap_mbox *mbox = mbox_chan_to_omap_mbox(chan);
+ struct omap_mbox *mbox = chan->con_priv;
struct omap_mbox_device *mdev = mbox->parent;
int ret = 0;
@@ -372,7 +364,7 @@ static int omap_mbox_chan_startup(struct mbox_chan *chan)
static void omap_mbox_chan_shutdown(struct mbox_chan *chan)
{
- struct omap_mbox *mbox = mbox_chan_to_omap_mbox(chan);
+ struct omap_mbox *mbox = chan->con_priv;
struct omap_mbox_device *mdev = mbox->parent;
mutex_lock(&mdev->cfg_lock);
@@ -415,7 +407,7 @@ static int omap_mbox_chan_send(struct omap_mbox *mbox, u32 msg)
static int omap_mbox_chan_send_data(struct mbox_chan *chan, void *data)
{
- struct omap_mbox *mbox = mbox_chan_to_omap_mbox(chan);
+ struct omap_mbox *mbox = chan->con_priv;
int ret;
u32 msg = (u32)(uintptr_t)(data);
--
2.39.2
The mbox_controller struct is only needed in the probe function. Make
it a local variable instead of storing a copy in omap_mbox_device
to simplify that struct.
Signed-off-by: Andrew Davis <[email protected]>
---
drivers/mailbox/omap-mailbox.c | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/mailbox/omap-mailbox.c b/drivers/mailbox/omap-mailbox.c
index 17c9b9df78b1d..97f59d9f9f319 100644
--- a/drivers/mailbox/omap-mailbox.c
+++ b/drivers/mailbox/omap-mailbox.c
@@ -86,7 +86,6 @@ struct omap_mbox_device {
u32 num_fifos;
u32 intr_type;
struct omap_mbox **mboxes;
- struct mbox_controller controller;
};
struct omap_mbox {
@@ -541,7 +540,7 @@ static struct mbox_chan *omap_mbox_of_xlate(struct mbox_controller *controller,
struct omap_mbox_device *mdev;
struct omap_mbox *mbox;
- mdev = container_of(controller, struct omap_mbox_device, controller);
+ mdev = dev_get_drvdata(controller->dev);
if (WARN_ON(!mdev))
return ERR_PTR(-EINVAL);
@@ -567,6 +566,7 @@ static int omap_mbox_probe(struct platform_device *pdev)
struct device_node *node = pdev->dev.of_node;
struct device_node *child;
const struct omap_mbox_match_data *match_data;
+ struct mbox_controller *controller;
u32 intr_type, info_count;
u32 num_users, num_fifos;
u32 tmp[3];
@@ -685,17 +685,20 @@ static int omap_mbox_probe(struct platform_device *pdev)
mdev->intr_type = intr_type;
mdev->mboxes = list;
+ controller = devm_kzalloc(&pdev->dev, sizeof(*controller), GFP_KERNEL);
+ if (!controller)
+ return -ENOMEM;
/*
* OMAP/K3 Mailbox IP does not have a Tx-Done IRQ, but rather a Tx-Ready
* IRQ and is needed to run the Tx state machine
*/
- mdev->controller.txdone_irq = true;
- mdev->controller.dev = mdev->dev;
- mdev->controller.ops = &omap_mbox_chan_ops;
- mdev->controller.chans = chnls;
- mdev->controller.num_chans = info_count;
- mdev->controller.of_xlate = omap_mbox_of_xlate;
- ret = devm_mbox_controller_register(mdev->dev, &mdev->controller);
+ controller->txdone_irq = true;
+ controller->dev = mdev->dev;
+ controller->ops = &omap_mbox_chan_ops;
+ controller->chans = chnls;
+ controller->num_chans = info_count;
+ controller->of_xlate = omap_mbox_of_xlate;
+ ret = devm_mbox_controller_register(mdev->dev, controller);
if (ret)
return ret;
--
2.39.2
Currently the driver loops through all mailbox child nodes twice, once
to read in data from each node, and again to make use of this data.
Instead read the data and make use of it in one pass. This removes
the need for several temporary data structures and reduces the
complexity of this main loop in probe.
Signed-off-by: Andrew Davis <[email protected]>
---
drivers/mailbox/omap-mailbox.c | 119 +++++++++++++--------------------
1 file changed, 46 insertions(+), 73 deletions(-)
diff --git a/drivers/mailbox/omap-mailbox.c b/drivers/mailbox/omap-mailbox.c
index 4f956c7b4072c..17c9b9df78b1d 100644
--- a/drivers/mailbox/omap-mailbox.c
+++ b/drivers/mailbox/omap-mailbox.c
@@ -89,19 +89,6 @@ struct omap_mbox_device {
struct mbox_controller controller;
};
-struct omap_mbox_fifo_info {
- int tx_id;
- int tx_usr;
- int tx_irq;
-
- int rx_id;
- int rx_usr;
- int rx_irq;
-
- const char *name;
- bool send_no_irq;
-};
-
struct omap_mbox {
const char *name;
int irq;
@@ -574,8 +561,7 @@ static int omap_mbox_probe(struct platform_device *pdev)
{
int ret;
struct mbox_chan *chnls;
- struct omap_mbox **list, *mbox, *mboxblk;
- struct omap_mbox_fifo_info *finfo, *finfoblk;
+ struct omap_mbox **list, *mbox;
struct omap_mbox_device *mdev;
struct omap_mbox_fifo *fifo;
struct device_node *node = pdev->dev.of_node;
@@ -609,40 +595,6 @@ static int omap_mbox_probe(struct platform_device *pdev)
return -ENODEV;
}
- finfoblk = devm_kcalloc(&pdev->dev, info_count, sizeof(*finfoblk),
- GFP_KERNEL);
- if (!finfoblk)
- return -ENOMEM;
-
- finfo = finfoblk;
- child = NULL;
- for (i = 0; i < info_count; i++, finfo++) {
- child = of_get_next_available_child(node, child);
- ret = of_property_read_u32_array(child, "ti,mbox-tx", tmp,
- ARRAY_SIZE(tmp));
- if (ret)
- return ret;
- finfo->tx_id = tmp[0];
- finfo->tx_irq = tmp[1];
- finfo->tx_usr = tmp[2];
-
- ret = of_property_read_u32_array(child, "ti,mbox-rx", tmp,
- ARRAY_SIZE(tmp));
- if (ret)
- return ret;
- finfo->rx_id = tmp[0];
- finfo->rx_irq = tmp[1];
- finfo->rx_usr = tmp[2];
-
- finfo->name = child->name;
-
- finfo->send_no_irq = of_property_read_bool(child, "ti,mbox-send-noirq");
-
- if (finfo->tx_id >= num_fifos || finfo->rx_id >= num_fifos ||
- finfo->tx_usr >= num_users || finfo->rx_usr >= num_users)
- return -EINVAL;
- }
-
mdev = devm_kzalloc(&pdev->dev, sizeof(*mdev), GFP_KERNEL);
if (!mdev)
return -ENOMEM;
@@ -667,36 +619,58 @@ static int omap_mbox_probe(struct platform_device *pdev)
if (!chnls)
return -ENOMEM;
- mboxblk = devm_kcalloc(&pdev->dev, info_count, sizeof(*mbox),
- GFP_KERNEL);
- if (!mboxblk)
- return -ENOMEM;
+ child = NULL;
+ for (i = 0; i < info_count; i++) {
+ int tx_id, tx_irq, tx_usr;
+ int rx_id, rx_usr;
+
+ mbox = devm_kzalloc(&pdev->dev, sizeof(*mbox), GFP_KERNEL);
+ if (!mbox)
+ return -ENOMEM;
+
+ child = of_get_next_available_child(node, child);
+ ret = of_property_read_u32_array(child, "ti,mbox-tx", tmp,
+ ARRAY_SIZE(tmp));
+ if (ret)
+ return ret;
+ tx_id = tmp[0];
+ tx_irq = tmp[1];
+ tx_usr = tmp[2];
+
+ ret = of_property_read_u32_array(child, "ti,mbox-rx", tmp,
+ ARRAY_SIZE(tmp));
+ if (ret)
+ return ret;
+ rx_id = tmp[0];
+ /* rx_irq = tmp[1]; */
+ rx_usr = tmp[2];
+
+ if (tx_id >= num_fifos || rx_id >= num_fifos ||
+ tx_usr >= num_users || rx_usr >= num_users)
+ return -EINVAL;
- mbox = mboxblk;
- finfo = finfoblk;
- for (i = 0; i < info_count; i++, finfo++) {
fifo = &mbox->tx_fifo;
- fifo->msg = MAILBOX_MESSAGE(finfo->tx_id);
- fifo->fifo_stat = MAILBOX_FIFOSTATUS(finfo->tx_id);
- fifo->intr_bit = MAILBOX_IRQ_NOTFULL(finfo->tx_id);
- fifo->irqenable = MAILBOX_IRQENABLE(intr_type, finfo->tx_usr);
- fifo->irqstatus = MAILBOX_IRQSTATUS(intr_type, finfo->tx_usr);
- fifo->irqdisable = MAILBOX_IRQDISABLE(intr_type, finfo->tx_usr);
+ fifo->msg = MAILBOX_MESSAGE(tx_id);
+ fifo->fifo_stat = MAILBOX_FIFOSTATUS(tx_id);
+ fifo->intr_bit = MAILBOX_IRQ_NOTFULL(tx_id);
+ fifo->irqenable = MAILBOX_IRQENABLE(intr_type, tx_usr);
+ fifo->irqstatus = MAILBOX_IRQSTATUS(intr_type, tx_usr);
+ fifo->irqdisable = MAILBOX_IRQDISABLE(intr_type, tx_usr);
fifo = &mbox->rx_fifo;
- fifo->msg = MAILBOX_MESSAGE(finfo->rx_id);
- fifo->msg_stat = MAILBOX_MSGSTATUS(finfo->rx_id);
- fifo->intr_bit = MAILBOX_IRQ_NEWMSG(finfo->rx_id);
- fifo->irqenable = MAILBOX_IRQENABLE(intr_type, finfo->rx_usr);
- fifo->irqstatus = MAILBOX_IRQSTATUS(intr_type, finfo->rx_usr);
- fifo->irqdisable = MAILBOX_IRQDISABLE(intr_type, finfo->rx_usr);
-
- mbox->send_no_irq = finfo->send_no_irq;
+ fifo->msg = MAILBOX_MESSAGE(rx_id);
+ fifo->msg_stat = MAILBOX_MSGSTATUS(rx_id);
+ fifo->intr_bit = MAILBOX_IRQ_NEWMSG(rx_id);
+ fifo->irqenable = MAILBOX_IRQENABLE(intr_type, rx_usr);
+ fifo->irqstatus = MAILBOX_IRQSTATUS(intr_type, rx_usr);
+ fifo->irqdisable = MAILBOX_IRQDISABLE(intr_type, rx_usr);
+
+ mbox->send_no_irq = of_property_read_bool(child, "ti,mbox-send-noirq");
mbox->intr_type = intr_type;
mbox->parent = mdev;
- mbox->name = finfo->name;
- mbox->irq = platform_get_irq(pdev, finfo->tx_irq);
+ mbox->name = child->name;
+ mbox->irq = platform_get_irq(pdev, tx_irq);
if (mbox->irq < 0)
return mbox->irq;
mbox->chan = &chnls[i];
@@ -743,7 +717,6 @@ static int omap_mbox_probe(struct platform_device *pdev)
if (ret < 0 && ret != -ENOSYS)
return ret;
- devm_kfree(&pdev->dev, finfoblk);
return 0;
}
--
2.39.2
This is only used internal to the driver, move it out of the
public header and into the driver file. While we are here,
this is not used as a bitwise, so drop that and make it a
simple enum type.
Signed-off-by: Andrew Davis <[email protected]>
---
drivers/mailbox/omap-mailbox.c | 5 +++++
include/linux/omap-mailbox.h | 4 ----
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/mailbox/omap-mailbox.c b/drivers/mailbox/omap-mailbox.c
index 8151722eef383..c083734b6954c 100644
--- a/drivers/mailbox/omap-mailbox.c
+++ b/drivers/mailbox/omap-mailbox.c
@@ -51,6 +51,11 @@
#define MBOX_INTR_CFG_TYPE1 0
#define MBOX_INTR_CFG_TYPE2 1
+typedef enum {
+ IRQ_TX = 1,
+ IRQ_RX = 2,
+} omap_mbox_irq_t;
+
struct omap_mbox_fifo {
unsigned long msg;
unsigned long fifo_stat;
diff --git a/include/linux/omap-mailbox.h b/include/linux/omap-mailbox.h
index f8ddf8e814167..3cc5c4ed7f5a6 100644
--- a/include/linux/omap-mailbox.h
+++ b/include/linux/omap-mailbox.h
@@ -10,8 +10,4 @@ typedef uintptr_t mbox_msg_t;
#define omap_mbox_message(data) (u32)(mbox_msg_t)(data)
-typedef int __bitwise omap_mbox_irq_t;
-#define IRQ_TX ((__force omap_mbox_irq_t) 1)
-#define IRQ_RX ((__force omap_mbox_irq_t) 2)
-
#endif /* OMAP_MAILBOX_H */
--
2.39.2