From fe19823f6053fe91a671fdbf92ab93b517a25219 Mon Sep 17 00:00:00 2001 From: Phil Elwell Date: Tue, 3 Nov 2020 11:49:53 +0000 Subject: [PATCH] Revert "mailbox: avoid timer start from callback" This reverts commit c7dacf5b0f32957b24ef29df1207dc2cd8307743. The Pi 400 shutdown/poweroff mechanism relies on being able to set a GPIO on the expander in the pm_power_off handler, something that requires two mailbox calls - GET_GPIO_STATE and SET_GPIO_STATE. A recent kernel change introduces a reasonable possibility that the GET call doesn't completes, and bisecting led to a commit from October that changes the timer usage of the mailbox. My theory is that there is a race condition in the new code that breaks the poll timer, but that it normally goes unnoticed because subsequent mailbox activity wakes it up again. The power-off mailbox calls happen at a time when other subsystems have been shut down, so if one of them fails then there is nothing to allow it to recover. See: https://github.com/raspberrypi/linux/issues/3941 Signed-off-by: Phil Elwell --- drivers/mailbox/mailbox.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) --- a/drivers/mailbox/mailbox.c +++ b/drivers/mailbox/mailbox.c @@ -82,12 +82,9 @@ static void msg_submit(struct mbox_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); - } + if (!err && (chan->txdone_method & TXDONE_BY_POLL)) + /* kick start the timer immediately to avoid delays */ + hrtimer_start(&chan->mbox->poll_hrt, 0, HRTIMER_MODE_REL); } static void tx_tick(struct mbox_chan *chan, int r) @@ -125,10 +122,11 @@ static enum hrtimer_restart txdone_hrtim 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; } }