mailbox: imx: Add a channel shutdown field

sashiko complained about possible teardown problem. The scenario

 CPU 0                              CPU 1
  imx_mu_isr()                   imx_mu_shutdown()
                                   imx_mu_xcr_rmw(priv, IMX_MU_RCR, 0, IMX_MU_xCR_RIEn(priv->dcfg->type, cp->idx));
    imx_mu_specific_rx()
      imx_mu_xcr_rmw(priv, IMX_MU_RCR, IMX_MU_xCR_RIEn(priv->dcfg->type, 0), 0);
                                   free_irq()

The RX event remains enabled because in this short window the RX event
was disabled in ->shutdown() while the interrupt was active and then
enabled again by the ISR while ->shutdown waited in free_irq().

This race requires timing and if happens can be problematic on shared
handlers if the "removed" channel triggers an interrupt. In this case
the irq-core will shutdown the interrupt with the "nobody cared"
message.

Introduce imx_mu_con_priv::shutdown to signal that the channel is
shutting down. This flag is set with the lock held (by
imx_mu_xcr_clr_shut()). The unmask side uses imx_mu_xcr_set_act() which
only enables the event if the channel has not been shutdown and
serialises on the same lock.

Reviewed-by: Peng Fan <peng.fan@nxp.com>
Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Jassi Brar <jassisinghbrar@gmail.com>

diff --git a/drivers/mailbox/imx-mailbox.c b/drivers/mailbox/imx-mailbox.c
index 0028073be4a71767138283e3c34bb8be59259206..e261b382d5c90a75fc2e8ca7171403c511c0df28 100644 (file)
--- a/drivers/mailbox/imx-mailbox.c
+++ b/drivers/mailbox/imx-mailbox.c
@@ -80,6 +80,7 @@ struct imx_mu_con_priv {
        enum imx_mu_chan_type   type;
        struct mbox_chan        *chan;
        struct work_struct      txdb_work;
+       bool                    shutdown;
 };
 
 struct imx_mu_priv {
@@ -219,6 +220,36 @@ static u32 imx_mu_xcr_rmw(struct imx_mu_priv *priv, enum imx_mu_xcr type, u32 se
        return val;
 }
 
+static void imx_mu_xcr_clr_shut(struct imx_mu_priv *priv, struct imx_mu_con_priv *cp,
+                               enum imx_mu_xcr type, u32 clr)
+{
+       unsigned long flags;
+       u32 val;
+
+       spin_lock_irqsave(&priv->xcr_lock, flags);
+       cp->shutdown = true;
+
+       val = imx_mu_read(priv, priv->dcfg->xCR[type]);
+       val &= ~clr;
+       imx_mu_write(priv, val, priv->dcfg->xCR[type]);
+       spin_unlock_irqrestore(&priv->xcr_lock, flags);
+}
+
+static void imx_mu_xcr_set_act(struct imx_mu_priv *priv, struct imx_mu_con_priv *cp,
+                              enum imx_mu_xcr type, u32 set)
+{
+       unsigned long flags;
+       u32 val;
+
+       spin_lock_irqsave(&priv->xcr_lock, flags);
+       if (!cp->shutdown) {
+               val = imx_mu_read(priv, priv->dcfg->xCR[type]);
+               val |= set;
+               imx_mu_write(priv, val, priv->dcfg->xCR[type]);
+       }
+       spin_unlock_irqrestore(&priv->xcr_lock, flags);
+}
+
 static int imx_mu_generic_tx(struct imx_mu_priv *priv,
                             struct imx_mu_con_priv *cp,
                             void *data)
@@ -377,7 +408,7 @@ static int imx_mu_specific_rx(struct imx_mu_priv *priv, struct imx_mu_con_priv *
                *data++ = imx_mu_read(priv, priv->dcfg->xRR + (i % num_rr) * 4);
        }
 
-       imx_mu_xcr_rmw(priv, IMX_MU_RCR, IMX_MU_xCR_RIEn(priv->dcfg->type, 0), 0);
+       imx_mu_xcr_set_act(priv, cp, IMX_MU_RCR, IMX_MU_xCR_RIEn(priv->dcfg->type, 0));
        mbox_chan_received_data(cp->chan, (void *)priv->msg);
 
        return 0;
@@ -605,6 +636,7 @@ static int imx_mu_startup(struct mbox_chan *chan)
                return ret;
        }
 
+       cp->shutdown = false;
        switch (cp->type) {
        case IMX_MU_TYPE_RX:
                imx_mu_xcr_rmw(priv, IMX_MU_RCR, IMX_MU_xCR_RIEn(priv->dcfg->type, cp->idx), 0);
@@ -639,13 +671,13 @@ static void imx_mu_shutdown(struct mbox_chan *chan)
 
        switch (cp->type) {
        case IMX_MU_TYPE_TX:
-               imx_mu_xcr_rmw(priv, IMX_MU_TCR, 0, IMX_MU_xCR_TIEn(priv->dcfg->type, cp->idx));
+               imx_mu_xcr_clr_shut(priv, cp, IMX_MU_TCR, IMX_MU_xCR_TIEn(priv->dcfg->type, cp->idx));
                break;
        case IMX_MU_TYPE_RX:
-               imx_mu_xcr_rmw(priv, IMX_MU_RCR, 0, IMX_MU_xCR_RIEn(priv->dcfg->type, cp->idx));
+               imx_mu_xcr_clr_shut(priv, cp, IMX_MU_RCR, IMX_MU_xCR_RIEn(priv->dcfg->type, cp->idx));
                break;
        case IMX_MU_TYPE_RXDB:
-               imx_mu_xcr_rmw(priv, IMX_MU_GIER, 0, IMX_MU_xCR_GIEn(priv->dcfg->type, cp->idx));
+               imx_mu_xcr_clr_shut(priv, cp, IMX_MU_GIER, IMX_MU_xCR_GIEn(priv->dcfg->type, cp->idx));
                break;
        case IMX_MU_TYPE_RST:
                imx_mu_xcr_rmw(priv, IMX_MU_CR, IMX_MU_xCR_RST(priv->dcfg->type), 0);