soundwire: cadence: fix race condition between suspend and Slave device alerts

In system suspend stress cases, the SOF CI reports timeouts. The root
cause is that an alert is generated while the system suspends. The
interrupt handling generates transactions on the bus that will never
be handled because the interrupts are disabled in parallel.

As a result, the transaction never completes and times out on resume.
This error doesn't seem too problematic since it happens in a work
queue, and the system recovers without issues.

Nevertheless, this race condition should not happen. When doing a
system suspend, or when disabling interrupts, we should make sure the
current transaction can complete, and prevent new work from being
queued.

BugLink: https://github.com/thesofproject/linux/issues/2344
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Acked-by: Jaroslav Kysela <perex@perex.cz>
Link: https://lore.kernel.org/r/20200817222340.18042-1-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>

diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c
index ecf503fb23e1566f711159e679140ec5ac575041..8c354f9468797062649fd81b1457b518f8170aa4 100644 (file)
--- a/drivers/soundwire/cadence_master.c
+++ b/drivers/soundwire/cadence_master.c
@@ -790,7 +790,16 @@ irqreturn_t sdw_cdns_irq(int irq, void *dev_id)
                             CDNS_MCP_INT_SLAVE_MASK, 0);
 
                int_status &= ~CDNS_MCP_INT_SLAVE_MASK;
-               schedule_work(&cdns->work);
+
+               /*
+                * Deal with possible race condition between interrupt
+                * handling and disabling interrupts on suspend.
+                *
+                * If the master is in the process of disabling
+                * interrupts, don't schedule a workqueue
+                */
+               if (cdns->interrupt_enabled)
+                       schedule_work(&cdns->work);
        }
 
        cdns_writel(cdns, CDNS_MCP_INTSTAT, int_status);
@@ -923,6 +932,19 @@ update_masks:
                slave_state = cdns_readl(cdns, CDNS_MCP_SLAVE_INTSTAT1);
                cdns_writel(cdns, CDNS_MCP_SLAVE_INTSTAT1, slave_state);
        }
+       cdns->interrupt_enabled = state;
+
+       /*
+        * Complete any on-going status updates before updating masks,
+        * and cancel queued status updates.
+        *
+        * There could be a race with a new interrupt thrown before
+        * the 3 mask updates below are complete, so in the interrupt
+        * we use the 'interrupt_enabled' status to prevent new work
+        * from being queued.
+        */
+       if (!state)
+               cancel_work_sync(&cdns->work);
 
        cdns_writel(cdns, CDNS_MCP_SLAVE_INTMASK0, slave_intmask0);
        cdns_writel(cdns, CDNS_MCP_SLAVE_INTMASK1, slave_intmask1);

diff --git a/drivers/soundwire/cadence_master.h b/drivers/soundwire/cadence_master.h
index fdec62b912d3b1a569be7161cb877e60a07d331d..4d1aab5b5ec2d6563617cde98d4a3986f73a5914 100644 (file)
--- a/drivers/soundwire/cadence_master.h
+++ b/drivers/soundwire/cadence_master.h
@@ -133,6 +133,7 @@ struct sdw_cdns {
 
        bool link_up;
        unsigned int msg_count;
+       bool interrupt_enabled;
 
        struct work_struct work;