mailbox: forward the hrtimer if not queued and under a lock

This reverts commit c7dacf5b0f32957b24ef29df1207dc2cd8307743,
"mailbox: avoid timer start from callback"

The previous commit was reverted since it lead to a race that
caused the hrtimer to not be started at all. The check for
hrtimer_active() in msg_submit() will return true if the
callback function txdone_hrtimer() is currently running. This
function could return HRTIMER_NORESTART and then the timer
will not be restarted, and also msg_submit() will not start
the timer. This will lead to a message actually being submitted
but no timer will start to check for its compleation.

The original fix that added checking hrtimer_active() was added to
avoid a warning with hrtimer_forward. Looking in the kernel
another solution to avoid this warning is to check hrtimer_is_queued()
before calling hrtimer_forward_now() instead. This however requires a
lock so the timer is not started by msg_submit() inbetween this check
and the hrtimer_forward() call.

Fixes: c7dacf5b0f32 ("mailbox: avoid timer start from callback")
Signed-off-by: Björn Ardö <bjorn.ardo@axis.com>
Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>

diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
index 3e7d4b20ab34fa852f5582105b08cc5dc318819c..4229b9b5da98f865ec36226c3f31310a70c96158 100644 (file)
--- a/drivers/mailbox/mailbox.c
+++ b/drivers/mailbox/mailbox.c
@@ -82,11 +82,11 @@ static void msg_submit(struct mbox_chan *chan)
 exit:
        spin_unlock_irqrestore(&chan->lock, flags);
 
-       /* kick start the timer immediately to avoid delays */
        if (!err && (chan->txdone_method & TXDONE_BY_POLL)) {
-               /* but only if not already active */
-               if (!hrtimer_active(&chan->mbox->poll_hrt))
-                       hrtimer_start(&chan->mbox->poll_hrt, 0, HRTIMER_MODE_REL);
+               /* kick start the timer immediately to avoid delays */
+               spin_lock_irqsave(&chan->mbox->poll_hrt_lock, flags);
+               hrtimer_start(&chan->mbox->poll_hrt, 0, HRTIMER_MODE_REL);
+               spin_unlock_irqrestore(&chan->mbox->poll_hrt_lock, flags);
        }
 }
 
@@ -120,20 +120,26 @@ static enum hrtimer_restart txdone_hrtimer(struct hrtimer *hrtimer)
                container_of(hrtimer, struct mbox_controller, poll_hrt);
        bool txdone, resched = false;
        int i;
+       unsigned long flags;
 
        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);
+                       else
+                               resched = true;
                }
        }
 
        if (resched) {
-               hrtimer_forward_now(hrtimer, ms_to_ktime(mbox->txpoll_period));
+               spin_lock_irqsave(&mbox->poll_hrt_lock, flags);
+               if (!hrtimer_is_queued(hrtimer))
+                       hrtimer_forward_now(hrtimer, ms_to_ktime(mbox->txpoll_period));
+               spin_unlock_irqrestore(&mbox->poll_hrt_lock, flags);
+
                return HRTIMER_RESTART;
        }
        return HRTIMER_NORESTART;
@@ -500,6 +506,7 @@ int mbox_controller_register(struct mbox_controller *mbox)
                hrtimer_init(&mbox->poll_hrt, CLOCK_MONOTONIC,
                             HRTIMER_MODE_REL);
                mbox->poll_hrt.function = txdone_hrtimer;
+               spin_lock_init(&mbox->poll_hrt_lock);
        }
 
        for (i = 0; i < mbox->num_chans; i++) {

diff --git a/include/linux/mailbox_controller.h b/include/linux/mailbox_controller.h
index 36d6ce673503c160d8c340ad3a56601d066a0a36..6fee33cb52f585f72de2c904f65c3e8f8887d11c 100644 (file)
--- a/include/linux/mailbox_controller.h
+++ b/include/linux/mailbox_controller.h
@@ -83,6 +83,7 @@ struct mbox_controller {
                                      const struct of_phandle_args *sp);
        /* Internal to API */
        struct hrtimer poll_hrt;
+       spinlock_t poll_hrt_lock;
        struct list_head node;
 };