From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
To: alsa-devel@alsa-project.org
Cc: linux-kernel@vger.kernel.org, tiwai@suse.de, broonie@kernel.org,
vkoul@kernel.org, gregkh@linuxfoundation.org, jank@cadence.com,
srinivas.kandagatla@linaro.org, slawomir.blauciak@intel.com,
Bard liao <yung-chuan.liao@linux.intel.com>,
Rander Wang <rander.wang@linux.intel.com>,
Ranjani Sridharan <ranjani.sridharan@linux.intel.com>,
Hui Wang <hui.wang@canonical.com>,
Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>,
Sanyog Kale <sanyog.r.kale@intel.com>
Subject: [PATCH 3/7] soundwire: intel: add mutex to prevent concurrent access to SHIM registers
Date: Wed, 11 Mar 2020 17:10:22 -0500 [thread overview]
Message-ID: <20200311221026.18174-4-pierre-louis.bossart@linux.intel.com> (raw)
In-Reply-To: <20200311221026.18174-1-pierre-louis.bossart@linux.intel.com>
Some of the SHIM registers exposed fields that are link specific, and
in addition some of the power-related registers (SPA/CPA) take time to
be updated. Uncontrolled access leads to timeouts or errors.
Add a mutex, shared by all links, so that all accesses to such
registers are serialized, and follow a pattern of read-modify-write.
The mutex initialization is done at the higher layer since the same
mutex is used for all links.
GitHub issue: https://github.com/thesofproject/linux/issues/1555
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
drivers/soundwire/intel.c | 37 +++++++++++++++++++++++++++++++------
drivers/soundwire/intel.h | 2 ++
2 files changed, 33 insertions(+), 6 deletions(-)
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 1a3b828b03a1..3c271a8044b8 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -286,6 +286,8 @@ static int intel_link_power_up(struct sdw_intel *sdw)
int spa_mask, cpa_mask;
int link_control, ret;
+ mutex_lock(sdw->link_res->shim_lock);
+
/* Link power up sequence */
link_control = intel_readl(shim, SDW_SHIM_LCTL);
spa_mask = (SDW_SHIM_LCTL_SPA << link_id);
@@ -293,6 +295,8 @@ static int intel_link_power_up(struct sdw_intel *sdw)
link_control |= spa_mask;
ret = intel_set_bit(shim, SDW_SHIM_LCTL, link_control, cpa_mask);
+ mutex_unlock(sdw->link_res->shim_lock);
+
if (ret < 0)
return ret;
@@ -307,6 +311,8 @@ static int intel_shim_init(struct sdw_intel *sdw)
int sync_reg, ret;
u16 ioctl = 0, act = 0;
+ mutex_lock(sdw->link_res->shim_lock);
+
/* Initialize Shim */
ioctl |= SDW_SHIM_IOCTL_BKE;
intel_writew(shim, SDW_SHIM_IOCTL(link_id), ioctl);
@@ -351,6 +357,8 @@ static int intel_shim_init(struct sdw_intel *sdw)
sync_reg |= SDW_SHIM_SYNC_SYNCCPU;
ret = intel_clear_bit(shim, SDW_SHIM_SYNC, sync_reg,
SDW_SHIM_SYNC_SYNCCPU);
+ mutex_unlock(sdw->link_res->shim_lock);
+
if (ret < 0)
dev_err(sdw->cdns.dev, "Failed to set sync period: %d\n", ret);
@@ -363,13 +371,15 @@ static void intel_shim_wake(struct sdw_intel *sdw, bool wake_enable)
unsigned int link_id = sdw->instance;
u16 wake_en, wake_sts;
+ mutex_lock(sdw->link_res->shim_lock);
+ wake_en = intel_readw(shim, SDW_SHIM_WAKEEN);
+
if (wake_enable) {
/* Enable the wakeup */
- intel_writew(shim, SDW_SHIM_WAKEEN,
- (SDW_SHIM_WAKEEN_ENABLE << link_id));
+ wake_en |= (SDW_SHIM_WAKEEN_ENABLE << link_id);
+ intel_writew(shim, SDW_SHIM_WAKEEN, wake_en);
} else {
/* Disable the wake up interrupt */
- wake_en = intel_readw(shim, SDW_SHIM_WAKEEN);
wake_en &= ~(SDW_SHIM_WAKEEN_ENABLE << link_id);
intel_writew(shim, SDW_SHIM_WAKEEN, wake_en);
@@ -378,6 +388,7 @@ static void intel_shim_wake(struct sdw_intel *sdw, bool wake_enable)
wake_sts |= (SDW_SHIM_WAKEEN_ENABLE << link_id);
intel_writew(shim, SDW_SHIM_WAKESTS_STATUS, wake_sts);
}
+ mutex_unlock(sdw->link_res->shim_lock);
}
static int intel_link_power_down(struct sdw_intel *sdw)
@@ -387,6 +398,8 @@ static int intel_link_power_down(struct sdw_intel *sdw)
void __iomem *shim = sdw->link_res->shim;
u16 ioctl;
+ mutex_lock(sdw->link_res->shim_lock);
+
/* Glue logic */
ioctl = intel_readw(shim, SDW_SHIM_IOCTL(link_id));
ioctl |= SDW_SHIM_IOCTL_BKE;
@@ -403,6 +416,8 @@ static int intel_link_power_down(struct sdw_intel *sdw)
link_control &= spa_mask;
ret = intel_clear_bit(shim, SDW_SHIM_LCTL, link_control, cpa_mask);
+ mutex_unlock(sdw->link_res->shim_lock);
+
if (ret < 0)
return ret;
@@ -630,11 +645,15 @@ static int intel_pre_bank_switch(struct sdw_bus *bus)
if (!bus->multi_link)
return 0;
+ mutex_lock(sdw->link_res->shim_lock);
+
/* Read SYNC register */
sync_reg = intel_readl(shim, SDW_SHIM_SYNC);
sync_reg |= SDW_SHIM_SYNC_CMDSYNC << sdw->instance;
intel_writel(shim, SDW_SHIM_SYNC, sync_reg);
+ mutex_unlock(sdw->link_res->shim_lock);
+
return 0;
}
@@ -649,6 +668,8 @@ static int intel_post_bank_switch(struct sdw_bus *bus)
if (!bus->multi_link)
return 0;
+ mutex_lock(sdw->link_res->shim_lock);
+
/* Read SYNC register */
sync_reg = intel_readl(shim, SDW_SHIM_SYNC);
@@ -660,9 +681,10 @@ static int intel_post_bank_switch(struct sdw_bus *bus)
*
* So, set the SYNCGO bit only if CMDSYNC bit is set for any Master.
*/
- if (!(sync_reg & SDW_SHIM_SYNC_CMDSYNC_MASK))
- return 0;
-
+ if (!(sync_reg & SDW_SHIM_SYNC_CMDSYNC_MASK)) {
+ ret = 0;
+ goto unlock;
+ }
/*
* Set SyncGO bit to synchronously trigger a bank switch for
* all the masters. A write to SYNCGO bit clears CMDSYNC bit for all
@@ -672,6 +694,9 @@ static int intel_post_bank_switch(struct sdw_bus *bus)
ret = intel_clear_bit(shim, SDW_SHIM_SYNC, sync_reg,
SDW_SHIM_SYNC_SYNCGO);
+unlock:
+ mutex_unlock(sdw->link_res->shim_lock);
+
if (ret < 0)
dev_err(sdw->cdns.dev, "Post bank switch failed: %d\n", ret);
diff --git a/drivers/soundwire/intel.h b/drivers/soundwire/intel.h
index 38b7c125fb10..568c84a80d79 100644
--- a/drivers/soundwire/intel.h
+++ b/drivers/soundwire/intel.h
@@ -15,6 +15,7 @@
* @irq: Interrupt line
* @ops: Shim callback ops
* @dev: device implementing hw_params and free callbacks
+ * @shim_lock: mutex to handle access to shared SHIM registers
*/
struct sdw_intel_link_res {
struct platform_device *pdev;
@@ -25,6 +26,7 @@ struct sdw_intel_link_res {
int irq;
const struct sdw_intel_ops *ops;
struct device *dev;
+ struct mutex *shim_lock; /* protect shared registers */
};
#endif /* __SDW_INTEL_LOCAL_H */
--
2.20.1
next prev parent reply other threads:[~2020-03-11 22:10 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-11 22:10 [PATCH 0/7] SoundWire: intel: fix SHIM programming sequences Pierre-Louis Bossart
2020-03-11 22:10 ` [PATCH 1/7] soundwire: intel: add helpers for link power down and shim wake Pierre-Louis Bossart
2020-03-11 22:10 ` [PATCH 2/7] soundwire: intel: reuse code for wait loops to set/clear bits Pierre-Louis Bossart
2020-03-20 13:38 ` Vinod Koul
2020-03-11 22:10 ` Pierre-Louis Bossart [this message]
2020-03-20 13:41 ` [PATCH 3/7] soundwire: intel: add mutex to prevent concurrent access to SHIM registers Vinod Koul
2020-03-20 14:07 ` Pierre-Louis Bossart
2020-03-23 12:18 ` Vinod Koul
2020-03-11 22:10 ` [PATCH 4/7] soundwire: intel: add definitions for shim_mask Pierre-Louis Bossart
2020-03-20 13:42 ` Vinod Koul
2020-03-20 14:13 ` Pierre-Louis Bossart
2020-03-23 12:28 ` Vinod Koul
2020-03-11 22:10 ` [PATCH 5/7] soundwire: intel: follow documentation sequences for SHIM registers Pierre-Louis Bossart
2020-03-20 13:51 ` Vinod Koul
2020-03-20 14:20 ` Pierre-Louis Bossart
2020-03-23 12:31 ` Vinod Koul
2020-03-11 22:10 ` [PATCH 6/7] soundwire: intel: introduce a helper to arm link synchronization Pierre-Louis Bossart
2020-03-11 22:10 ` [PATCH 7/7] soundwire: intel: introduce helper for " Pierre-Louis Bossart
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200311221026.18174-4-pierre-louis.bossart@linux.intel.com \
--to=pierre-louis.bossart@linux.intel.com \
--cc=alsa-devel@alsa-project.org \
--cc=broonie@kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=hui.wang@canonical.com \
--cc=jank@cadence.com \
--cc=linux-kernel@vger.kernel.org \
--cc=rander.wang@linux.intel.com \
--cc=ranjani.sridharan@linux.intel.com \
--cc=sanyog.r.kale@intel.com \
--cc=slawomir.blauciak@intel.com \
--cc=srinivas.kandagatla@linaro.org \
--cc=tiwai@suse.de \
--cc=vkoul@kernel.org \
--cc=yung-chuan.liao@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).