[PATCH 00/21] soundwire: Fix Critical and High issues reported by Sashiko

[PATCH 00/21] soundwire: Fix Critical and High issues reported by Sashiko

[Lee Jones]

These issues were reported by Sashiko.

Cc: "Uwe Kleine-König (The Capable Hub)" <u.kleine-koenig@baylibre.com>
Cc: Bard Liao <yung-chuan.liao@linux.intel.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Daniel Baluta <daniel.baluta@nxp.com>
Cc: Danilo Krummrich <dakr@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Hardik T Shah <hardik.t.shah@intel.com>
Cc: Jaroslav Kysela <perex@perex.cz>
Cc: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: linux-arm-msm@vger.kernel.org
Cc: linux-sound@vger.kernel.org
Cc: Mark Brown <broonie@kernel.org>
Cc: Mastan Katragadda <Mastan.Katragadda@amd.com>
Cc: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.dev>
Cc: Rander Wang <rander.wang@intel.com>
Cc: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Cc: Sanyog Kale <sanyog.r.kale@intel.com>
Cc: Simon Trimmer <simont@opensource.cirrus.com>
Cc: sound-open-firmware@alsa-project.org
Cc: Srinivas Kandagatla <srini@kernel.org>
Cc: Takashi Iwai <tiwai@suse.com>
Cc: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Cc: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
Cc: Vinod Koul <vkoul@kernel.org>

Lee Jones (21):
  soundwire: bus: Hold bus_lock when looking up slave devices
  soundwire: sysfs: Use sysfs_emit_at() in array show callbacks
  soundwire: stream: Prevent m_rt_count underflow in
    sdw_stream_remove_master()
  soundwire: stream: Clean up allocated runtime resources on config
    failure
  ASoC: SOF: Intel: hda-sdw-bpt: Stop active DMA before closing BPT
    streams
  soundwire: intel_ace2x: Protect bus.bpt_stream with bus_lock
  soundwire: qcom: Prevent pconfig buffer overflow in
    qcom_swrm_stream_alloc_ports()
  soundwire: qcom: Disable runtime PM and avoid clock underflow in
    qcom_swrm_remove()
  soundwire: qcom: Check clk_prepare_enable() return value
  soundwire: qcom: Serialise ctrl->intr_mask updates
  soundwire: qcom: Prevent stale broadcast IRQ from completing
    subsequent command
  soundwire: amd: Check for NULL manager drvdata in sdw_amd_startup()
  soundwire: amd: Check ping command response for timeout
  soundwire: amd: Preserve lane control bits in amd_sdw_port_enable()
  soundwire: amd: Avoid static port offset leak in
    amd_sdw_compute_params()
  soundwire: amd: Initialise and cancel IRQ work around manager lifetime
  soundwire: amd: Resume manager before hardware teardown in
    amd_sdw_manager_remove()
  soundwire: amd: Synchronise parent IRQ and disable work on manager
    removal
  soundwire: amd: Cancel pending work before suspending manager
  soundwire: amd: Check clock resume timeout in amd_resume_runtime()
  soundwire: amd: Disable interrupts if enabling manager fails

 drivers/soundwire/amd_init.c        |  13 ++-
 drivers/soundwire/amd_manager.c     | 130 +++++++++++++++++++++++-----
 drivers/soundwire/bus.c             |   6 +-
 drivers/soundwire/intel_ace2x.c     |  36 ++++----
 drivers/soundwire/qcom.c            |  97 +++++++++++++++++----
 drivers/soundwire/stream.c          |  40 ++++++---
 drivers/soundwire/sysfs_slave.c     |   6 +-
 drivers/soundwire/sysfs_slave_dpn.c |   8 +-
 sound/soc/sof/intel/hda-sdw-bpt.c   |   6 ++
 9 files changed, 267 insertions(+), 75 deletions(-)

-- 
2.56.0.317.g454f8302e1-goog

[PATCH 01/21] soundwire: bus: Hold bus_lock when looking up slave devices

[Lee Jones]

sdw_get_slave() iterates over bus->slaves and requires bus->bus_lock to
be held by the caller.  However, sdw_handle_slave_status() and
sdw_clear_slave_status() drop bus->bus_lock immediately before calling
sdw_get_slave().  If a concurrent slave teardown unlinks the node via
list_del_init() and frees the slave while bus->bus_lock is dropped,
sdw_get_slave() can spin forever on the self-referential list entry or
dereference freed memory.

Keep bus->bus_lock held across the sdw_get_slave() lookup in
sdw_handle_slave_status() and sdw_clear_slave_status().

Fixes: b0a9c37b0178 ("soundwire: Add slave status handling")
Closes: https://sashiko.dev/#/bug/linux-b0e65a93-dfd4-4e3d-a96c-8f13ba0aafd2
Cc: stable@vger.kernel.org
Signed-off-by: Lee Jones <lee@kernel.org>
---
 drivers/soundwire/bus.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index aeaae5a57c89..15c66effbf4f 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -1907,9 +1907,9 @@ int sdw_handle_slave_status(struct sdw_bus *bus,
 			mutex_unlock(&bus->bus_lock);
 			continue;
 		}
-		mutex_unlock(&bus->bus_lock);
 
 		slave = sdw_get_slave(bus, i);
+		mutex_unlock(&bus->bus_lock);
 		if (!slave)
 			continue;
 
@@ -1954,9 +1954,9 @@ int sdw_handle_slave_status(struct sdw_bus *bus,
 			mutex_unlock(&bus->bus_lock);
 			continue;
 		}
-		mutex_unlock(&bus->bus_lock);
 
 		slave = sdw_get_slave(bus, i);
+		mutex_unlock(&bus->bus_lock);
 		if (!slave)
 			continue;
 
@@ -2052,9 +2052,9 @@ void sdw_clear_slave_status(struct sdw_bus *bus, u32 request)
 			mutex_unlock(&bus->bus_lock);
 			continue;
 		}
-		mutex_unlock(&bus->bus_lock);
 
 		slave = sdw_get_slave(bus, i);
+		mutex_unlock(&bus->bus_lock);
 		if (!slave)
 			continue;
 
-- 
2.56.0.317.g454f8302e1-goog

[PATCH 02/21] soundwire: sysfs: Use sysfs_emit_at() in array show callbacks

[Lee Jones]

words_show() and the show callbacks generated by sdw_dpn_array_attr()
format firmware-supplied property arrays into the sysfs buffer using
unbounded sprintf(buf + size, ...) loops.  If firmware provides a large
array count, sprintf() can write past PAGE_SIZE and corrupt kernel
memory.

Replace the sprintf(buf + size, ...) calls with bounds-checked
sysfs_emit_at() calls.

Fixes: bcac59029955 ("soundwire: add Slave sysfs support")
Closes: https://sashiko.dev/#/bug/linux-543ff498-3bed-412d-8972-4855c5aa6008
Cc: stable@vger.kernel.org
Signed-off-by: Lee Jones <lee@kernel.org>
---
 drivers/soundwire/sysfs_slave.c     | 6 +++---
 drivers/soundwire/sysfs_slave_dpn.c | 8 ++++----
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/soundwire/sysfs_slave.c b/drivers/soundwire/sysfs_slave.c
index a2ee064147e0..35272cfe9fc8 100644
--- a/drivers/soundwire/sysfs_slave.c
+++ b/drivers/soundwire/sysfs_slave.c
@@ -161,9 +161,9 @@ static ssize_t words_show(struct device *dev,
 	int i;
 
 	for (i = 0; i < slave->prop.dp0_prop->num_words; i++)
-		size += sprintf(buf + size, "%d ",
-				slave->prop.dp0_prop->words[i]);
-	size += sprintf(buf + size, "\n");
+		size += sysfs_emit_at(buf, size, "%d ",
+				      slave->prop.dp0_prop->words[i]);
+	size += sysfs_emit_at(buf, size, "\n");
 
 	return size;
 }
diff --git a/drivers/soundwire/sysfs_slave_dpn.c b/drivers/soundwire/sysfs_slave_dpn.c
index d2561400da77..16872ea64b1f 100644
--- a/drivers/soundwire/sysfs_slave_dpn.c
+++ b/drivers/soundwire/sysfs_slave_dpn.c
@@ -134,10 +134,10 @@ static ssize_t field##_dpn_show(struct sdw_slave *slave,		\
 	for_each_set_bit(bit, &mask, 32) {				\
 		if (bit == N) {						\
 			for (j = 0; j < dpn[i].num_##field; j++)	\
-				size += sprintf(buf + size,		\
-						format_string,		\
-						dpn[i].field[j]);	\
-			size += sprintf(buf + size, "\n");		\
+				size += sysfs_emit_at(buf, size,	\
+						      format_string,	\
+						      dpn[i].field[j]);	\
+			size += sysfs_emit_at(buf, size, "\n");		\
 			return size;					\
 		}							\
 		i++;							\
-- 
2.56.0.317.g454f8302e1-goog

[PATCH 03/21] soundwire: stream: Prevent m_rt_count underflow in sdw_stream_remove_master()

[Lee Jones]

stream->m_rt_count is only incremented in sdw_stream_add_master() after
sdw_master_port_alloc() succeeds.  However, sdw_stream_add_slave() can
allocate a master runtime without allocating master ports, and
sdw_master_port_alloc() can fail after partially populating
m_rt->port_list.  When sdw_stream_remove_master() subsequently tears
down the stream, it unconditionally decrements stream->m_rt_count,
underflowing the counter when master ports were never allocated.

Free any partially allocated master ports on allocation failure in
sdw_master_port_alloc(), and only decrement stream->m_rt_count in
sdw_stream_remove_master() when master ports were allocated.

Fixes: ac3bc88cc56e ("soundwire: stream: separate alloc and config within sdw_stream_add_xxx()")
Closes: https://sashiko.dev/#/bug/linux-bce288e6-c97f-46dd-9fc3-023c33d56f36
Cc: stable@vger.kernel.org
Signed-off-by: Lee Jones <lee@kernel.org>
---
 drivers/soundwire/stream.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index 0b00fcdcab2d..f225e5438ccf 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1113,8 +1113,10 @@ static int sdw_master_port_alloc(struct sdw_master_runtime *m_rt,
 	/* Iterate for number of ports to perform initialization */
 	for (i = 0; i < num_ports; i++) {
 		p_rt = sdw_port_alloc(&m_rt->port_list);
-		if (!p_rt)
+		if (!p_rt) {
+			sdw_master_port_free(m_rt);
 			return -ENOMEM;
+		}
 	}
 
 	return 0;
@@ -2108,9 +2110,10 @@ int sdw_stream_remove_master(struct sdw_bus *bus,
 		if (m_rt->bus != bus)
 			continue;
 
+		if (sdw_master_port_allocated(m_rt))
+			stream->m_rt_count--;
 		sdw_master_port_free(m_rt);
 		sdw_master_rt_free(m_rt, stream);
-		stream->m_rt_count--;
 	}
 
 	if (list_empty(&stream->master_list))
-- 
2.56.0.317.g454f8302e1-goog

[PATCH 04/21] soundwire: stream: Clean up allocated runtime resources on config failure

[Lee Jones]

In sdw_stream_add_master() and sdw_stream_add_slave(), failures during
runtime or port configuration jump directly to the unlock label,
bypassing alloc_error and leaving newly allocated master/slave runtime
and port structures attached to the stream and bus lists.

Track whether ports were newly allocated in the current invocation and
route configuration failures through alloc_error so all resources
allocated by the failing call are released.

Fixes: ac3bc88cc56e ("soundwire: stream: separate alloc and config within sdw_stream_add_xxx()")
Closes: https://sashiko.dev/#/bug/linux-546e137d-6e4d-4f22-99c3-8ccc8beb5350
Cc: stable@vger.kernel.org
Signed-off-by: Lee Jones <lee@kernel.org>
---
 drivers/soundwire/stream.c | 33 +++++++++++++++++++++++++--------
 1 file changed, 25 insertions(+), 8 deletions(-)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index f225e5438ccf..97338a13c1a0 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1026,14 +1026,18 @@ static int sdw_slave_port_alloc(struct sdw_slave *slave,
 				struct sdw_slave_runtime *s_rt,
 				unsigned int num_config)
 {
-	struct sdw_port_runtime *p_rt;
+	struct sdw_port_runtime *p_rt, *_p_rt;
 	int i;
 
 	/* Iterate for number of ports to perform initialization */
 	for (i = 0; i < num_config; i++) {
 		p_rt = sdw_port_alloc(&s_rt->port_list);
-		if (!p_rt)
+		if (!p_rt) {
+			list_for_each_entry_safe(p_rt, _p_rt,
+						 &s_rt->port_list, port_node)
+				sdw_port_free(p_rt);
 			return -ENOMEM;
+		}
 	}
 
 	return 0;
@@ -2018,6 +2022,7 @@ int sdw_stream_add_master(struct sdw_bus *bus,
 			  struct sdw_stream_runtime *stream)
 {
 	struct sdw_master_runtime *m_rt;
+	bool alloc_master_port = false;
 	bool alloc_master_rt = false;
 	int ret;
 
@@ -2063,18 +2068,21 @@ int sdw_stream_add_master(struct sdw_bus *bus,
 		if (ret)
 			goto alloc_error;
 
+		alloc_master_port = true;
 		stream->m_rt_count++;
 	}
 
 	ret = sdw_master_rt_config(m_rt, stream_config);
 	if (ret < 0)
-		goto unlock;
+		goto alloc_error;
 
 	ret = sdw_config_stream(bus->dev, stream, stream_config, false);
 	if (ret)
-		goto unlock;
+		goto alloc_error;
 
 	ret = sdw_master_port_config(m_rt, port_config);
+	if (ret)
+		goto alloc_error;
 
 	goto unlock;
 
@@ -2082,6 +2090,10 @@ int sdw_stream_add_master(struct sdw_bus *bus,
 	/*
 	 * we only cleanup what was allocated in this routine
 	 */
+	if (alloc_master_port) {
+		sdw_master_port_free(m_rt);
+		stream->m_rt_count--;
+	}
 	if (alloc_master_rt)
 		sdw_master_rt_free(m_rt, stream);
 unlock:
@@ -2147,6 +2159,7 @@ int sdw_stream_add_slave(struct sdw_slave *slave,
 	struct sdw_slave_runtime *s_rt;
 	struct sdw_master_runtime *m_rt;
 	bool alloc_master_rt = false;
+	bool alloc_slave_port = false;
 	bool alloc_slave_rt = false;
 
 	int ret;
@@ -2197,24 +2210,26 @@ int sdw_stream_add_slave(struct sdw_slave *slave,
 		ret = sdw_slave_port_alloc(slave, s_rt, num_ports);
 		if (ret)
 			goto alloc_error;
+
+		alloc_slave_port = true;
 	}
 
 	ret =  sdw_master_rt_config(m_rt, stream_config);
 	if (ret)
-		goto unlock;
+		goto alloc_error;
 
 	ret = sdw_slave_rt_config(s_rt, stream_config);
 	if (ret)
-		goto unlock;
+		goto alloc_error;
 
 	ret = sdw_config_stream(&slave->dev, stream, stream_config, true);
 	if (ret)
-		goto unlock;
+		goto alloc_error;
 
 	ret = sdw_slave_port_config(slave, s_rt, port_config,
 				    stream->type == SDW_STREAM_BPT);
 	if (ret)
-		goto unlock;
+		goto alloc_error;
 
 	/*
 	 * Change stream state to CONFIGURED on first Slave add.
@@ -2231,6 +2246,8 @@ int sdw_stream_add_slave(struct sdw_slave *slave,
 	 * is intentional, the 'master_rt_free' will call sdw_slave_rt_free()
 	 * internally.
 	 */
+	if (alloc_slave_port)
+		sdw_slave_port_free(slave, stream);
 	if (alloc_master_rt)
 		sdw_master_rt_free(m_rt, stream);
 	else if (alloc_slave_rt)
-- 
2.56.0.317.g454f8302e1-goog

[PATCH 05/21] ASoC: SOF: Intel: hda-sdw-bpt: Stop active DMA before closing BPT streams

[Lee Jones]

When sdw_enable_stream() fails in intel_ace2x_bpt_send_async() after
hda_sdw_bpt_send_async() has already started the TX and RX DMA streams,
intel_ace2x_bpt_close_stream() invokes hda_sdw_bpt_close() directly
without calling hda_sdw_bpt_wait().  As a result,
hda_sdw_bpt_dma_deprepare() tears down the DMA buffers and BDL while
the DMA hardware is still running.

Disable any running TX or RX BPT DMA stream at the start of
hda_sdw_bpt_close() before unmapping channels and depreparing DMA
resources.

Fixes: 4c1ce9f37d8a ("soundwire: intel_ace2x: add BPT send_async/wait callbacks")
Closes: https://sashiko.dev/#/bug/linux-ff0012de-abdd-4796-875e-b1fbbf47235f
Cc: stable@vger.kernel.org
Signed-off-by: Lee Jones <lee@kernel.org>
---
 sound/soc/sof/intel/hda-sdw-bpt.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/sound/soc/sof/intel/hda-sdw-bpt.c b/sound/soc/sof/intel/hda-sdw-bpt.c
index ba9fbd662b85..9d97d8676251 100644
--- a/sound/soc/sof/intel/hda-sdw-bpt.c
+++ b/sound/soc/sof/intel/hda-sdw-bpt.c
@@ -456,6 +456,12 @@ int hda_sdw_bpt_close(struct device *dev, int link_id, struct hdac_ext_stream *b
 	int ret;
 	int ret1;
 
+	if (bpt_rx_stream && hdac_stream(bpt_rx_stream)->running)
+		hda_sdw_bpt_dma_disable(dev, bpt_rx_stream);
+
+	if (bpt_tx_stream && hdac_stream(bpt_tx_stream)->running)
+		hda_sdw_bpt_dma_disable(dev, bpt_tx_stream);
+
 	/*
 	 * In the case of SoundWire we need to reset the PCMSyCM registers.
 	 * Need to continue depreparing the DMA buffers even if this fails.
-- 
2.56.0.317.g454f8302e1-goog

[PATCH 06/21] soundwire: intel_ace2x: Protect bus.bpt_stream with bus_lock

[Lee Jones]

intel_ace2x_bpt_open_stream() checks and assigns cdns->bus.bpt_stream
without holding cdns->bus.bus_lock.  Concurrent BPT open requests can
both observe a NULL cdns->bus.bpt_stream pointer, allocate separate
streams, and overwrite cdns->bus.bpt_stream, leaking the first stream.
In addition, failures after sdw_slave_bpt_stream_add() jump past
sdw_stream_remove_master(), leaving the master runtime allocated by
sdw_stream_add_slave() on the bus list.

Hold cdns->bus.bus_lock when checking, assigning, and clearing
cdns->bus.bpt_stream, and remove both slave and master stream runtimes
on open failure.

Fixes: b2c9f1d5a7eb ("soundwire: Intel: test bus.bpt_stream before assigning it")
Closes: https://sashiko.dev/#/bug/linux-88535ed9-b7e9-4ede-93c7-59950c524c0b
Cc: stable@vger.kernel.org
Signed-off-by: Lee Jones <lee@kernel.org>
---
 drivers/soundwire/intel_ace2x.c | 36 ++++++++++++++++++---------------
 1 file changed, 20 insertions(+), 16 deletions(-)

diff --git a/drivers/soundwire/intel_ace2x.c b/drivers/soundwire/intel_ace2x.c
index 642b33ab5526..3623fc9f4451 100644
--- a/drivers/soundwire/intel_ace2x.c
+++ b/drivers/soundwire/intel_ace2x.c
@@ -83,17 +83,19 @@ static int intel_ace2x_bpt_open_stream(struct sdw_intel *sdw, struct sdw_slave *
 	int len;
 	int i;
 
-	if (cdns->bus.bpt_stream) {
-		dev_err(cdns->dev, "%s: BPT stream already exists\n", __func__);
-		return -EAGAIN;
+	scoped_guard(mutex, &cdns->bus.bus_lock) {
+		if (cdns->bus.bpt_stream) {
+			dev_err(cdns->dev, "%s: BPT stream already exists\n", __func__);
+			return -EAGAIN;
+		}
+
+		stream = sdw_alloc_stream("BPT", SDW_STREAM_BPT);
+		if (!stream)
+			return -ENOMEM;
+
+		cdns->bus.bpt_stream = stream;
 	}
 
-	stream = sdw_alloc_stream("BPT", SDW_STREAM_BPT);
-	if (!stream)
-		return -ENOMEM;
-
-	cdns->bus.bpt_stream = stream;
-
 	ret = sdw_slave_bpt_stream_add(slave, stream);
 	if (ret < 0)
 		goto release_stream;
@@ -105,7 +107,7 @@ static int intel_ace2x_bpt_open_stream(struct sdw_intel *sdw, struct sdw_slave *
 	if (!pdi0) {
 		dev_err(cdns->dev, "%s: sdw_cdns_alloc_pdi0 failed\n", __func__);
 		ret = -EINVAL;
-		goto remove_slave;
+		goto remove_master;
 	}
 
 	sdw_cdns_config_stream(cdns, 1, dir, pdi0);
@@ -117,7 +119,7 @@ static int intel_ace2x_bpt_open_stream(struct sdw_intel *sdw, struct sdw_slave *
 	if (!pdi1) {
 		dev_err(cdns->dev, "%s: sdw_cdns_alloc_pdi1 failed\n", __func__);
 		ret = -EINVAL;
-		goto remove_slave;
+		goto remove_master;
 	}
 
 	sdw_cdns_config_stream(cdns, 1, dir, pdi1);
@@ -136,7 +138,7 @@ static int intel_ace2x_bpt_open_stream(struct sdw_intel *sdw, struct sdw_slave *
 	pconfig = kzalloc_objs(*pconfig, 2);
 	if (!pconfig) {
 		ret =  -ENOMEM;
-		goto remove_slave;
+		goto remove_master;
 	}
 
 	for (i = 0; i < 2 /* num_pdi */; i++) {
@@ -149,7 +151,7 @@ static int intel_ace2x_bpt_open_stream(struct sdw_intel *sdw, struct sdw_slave *
 
 	if (ret < 0) {
 		dev_err(cdns->dev, "add master to stream failed:%d\n", ret);
-		goto remove_slave;
+		goto remove_master;
 	}
 
 	ret = sdw_prepare_stream(cdns->bus.bpt_stream);
@@ -293,15 +295,16 @@ static int intel_ace2x_bpt_open_stream(struct sdw_intel *sdw, struct sdw_slave *
 		dev_err(cdns->dev, "%s: remove master failed: %d\n",
 			__func__, ret1);
 
-remove_slave:
 	ret1 = sdw_stream_remove_slave(slave, cdns->bus.bpt_stream);
 	if (ret1 < 0)
 		dev_err(cdns->dev, "%s: remove slave failed: %d\n",
 			__func__, ret1);
 
 release_stream:
-	sdw_release_stream(cdns->bus.bpt_stream);
-	cdns->bus.bpt_stream = NULL;
+	scoped_guard(mutex, &cdns->bus.bus_lock) {
+		sdw_release_stream(cdns->bus.bpt_stream);
+		cdns->bus.bpt_stream = NULL;
+	}
 
 	return ret;
 }
@@ -335,6 +338,7 @@ static void intel_ace2x_bpt_close_stream(struct sdw_intel *sdw, struct sdw_slave
 		dev_err(cdns->dev, "%s: remove slave failed: %d\n",
 			__func__, ret);
 
+	guard(mutex)(&cdns->bus.bus_lock);
 	sdw_release_stream(cdns->bus.bpt_stream);
 	cdns->bus.bpt_stream = NULL;
 }
-- 
2.56.0.317.g454f8302e1-goog

[PATCH 07/21] soundwire: qcom: Prevent pconfig buffer overflow in qcom_swrm_stream_alloc_ports()

[Lee Jones]

qcom_swrm_stream_alloc_ports() allocates the pconfig array with
ctrl->nports elements, but only checks that the assigned port number pn
is below maxport.  When multiple slave ports map to the same master port
via m_port_map or the stream requests more slave ports than
ctrl->nports, nports increments past ctrl->nports and writes out of
bounds of pconfig.

Verify that nports is less than maxport before writing to
pconfig[nports].

Fixes: 9e53a66a2f2f ("soundwire: qcom: deprecate qcom,din/out-ports")
Closes: https://sashiko.dev/#/bug/linux-543ea0c0-a6e5-4b03-a041-ea0a760802c1
Cc: stable@vger.kernel.org
Signed-off-by: Lee Jones <lee@kernel.org>
---
 drivers/soundwire/qcom.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
index 6e0c5fb10efa..205011234030 100644
--- a/drivers/soundwire/qcom.c
+++ b/drivers/soundwire/qcom.c
@@ -1343,6 +1343,11 @@ static int qcom_swrm_stream_alloc_ports(struct qcom_swrm_ctrl *ctrl,
 		list_for_each_entry(s_rt, &m_rt->slave_rt_list, m_rt_node) {
 			slave = s_rt->slave;
 			list_for_each_entry(p_rt, &s_rt->port_list, port_node) {
+				if (nports >= maxport) {
+					dev_err(ctrl->dev, "All ports busy\n");
+					return -EBUSY;
+				}
+
 				m_port = slave->m_port_map[p_rt->num];
 				/* Port numbers start from 1 - 14*/
 				if (m_port)
-- 
2.56.0.317.g454f8302e1-goog

[PATCH 08/21] soundwire: qcom: Disable runtime PM and avoid clock underflow in qcom_swrm_remove()

[Lee Jones]

When the controller is runtime-suspended, swrm_runtime_suspend() has
already disabled ctrl->hclk.  If the device is unbound in that state,
qcom_swrm_remove() omits pm_runtime_disable() and unconditionally calls
clk_disable_unprepare(), triggering a clock enable count underflow and
leaving runtime PM enabled.

Resume the device before removing the bus master, disable runtime PM,
and only disable ctrl->hclk if the device is not runtime-suspended.

Fixes: 74e79da9fd46 ("soundwire: qcom: add runtime pm support")
Closes: https://sashiko.dev/#/bug/linux-e26e04cd-3dec-4a3d-94e6-f45403c4bb57
Cc: stable@vger.kernel.org
Signed-off-by: Lee Jones <lee@kernel.org>
---
 drivers/soundwire/qcom.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
index 205011234030..246bc063b850 100644
--- a/drivers/soundwire/qcom.c
+++ b/drivers/soundwire/qcom.c
@@ -1791,8 +1791,13 @@ static void qcom_swrm_remove(struct platform_device *pdev)
 {
 	struct qcom_swrm_ctrl *ctrl = dev_get_drvdata(&pdev->dev);
 
+	pm_runtime_get_sync(&pdev->dev);
 	sdw_bus_master_delete(&ctrl->bus);
-	clk_disable_unprepare(ctrl->hclk);
+	pm_runtime_disable(&pdev->dev);
+	pm_runtime_put_noidle(&pdev->dev);
+	if (!pm_runtime_status_suspended(&pdev->dev))
+		clk_disable_unprepare(ctrl->hclk);
+	pm_runtime_set_suspended(&pdev->dev);
 }
 
 static int __maybe_unused swrm_runtime_resume(struct device *dev)
-- 
2.56.0.317.g454f8302e1-goog

[PATCH 09/21] soundwire: qcom: Check clk_prepare_enable() return value

[Lee Jones]

qcom_swrm_probe(), qcom_swrm_irq_handler(), and swrm_runtime_resume()
ignore the return value of clk_prepare_enable(ctrl->hclk) and proceed
to access controller registers even when enabling the interface clock
fails.  Also jump to err_clk when requesting the wake IRQ fails in
qcom_swrm_probe() so ctrl->hclk is disabled on error.

Check the return value of clk_prepare_enable() in all three callers and
abort register access on failure.

Fixes: 02efb49aa805 ("soundwire: qcom: add support for SoundWire controller")
Closes: https://sashiko.dev/#/bug/linux-f65cddc9-08ff-4531-9acf-76981d74d914
Cc: stable@vger.kernel.org
Signed-off-by: Lee Jones <lee@kernel.org>
---
 drivers/soundwire/qcom.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
index 246bc063b850..555045b32a25 100644
--- a/drivers/soundwire/qcom.c
+++ b/drivers/soundwire/qcom.c
@@ -778,7 +778,9 @@ static irqreturn_t qcom_swrm_irq_handler(int irq, void *dev_id)
 	u32 i;
 	int devnum;
 	int ret = IRQ_HANDLED;
-	clk_prepare_enable(ctrl->hclk);
+
+	if (clk_prepare_enable(ctrl->hclk))
+		return IRQ_NONE;
 
 	ctrl->reg_read(ctrl, ctrl->reg_layout[SWRM_REG_INTERRUPT_STATUS],
 		       &intr_sts);
@@ -1680,7 +1682,9 @@ static int qcom_swrm_probe(struct platform_device *pdev)
 		goto err_init;
 	}
 
-	clk_prepare_enable(ctrl->hclk);
+	ret = clk_prepare_enable(ctrl->hclk);
+	if (ret)
+		goto err_init;
 
 	ctrl->dev = dev;
 	dev_set_drvdata(&pdev->dev, ctrl);
@@ -1736,7 +1740,7 @@ static int qcom_swrm_probe(struct platform_device *pdev)
 						"swr_wake_irq", ctrl);
 		if (ret) {
 			dev_err(dev, "Failed to request soundwire wake irq\n");
-			goto err_init;
+			goto err_clk;
 		}
 	}
 
@@ -1805,13 +1809,15 @@ static int __maybe_unused swrm_runtime_resume(struct device *dev)
 	struct qcom_swrm_ctrl *ctrl = dev_get_drvdata(dev);
 	int ret;
 
+	ret = clk_prepare_enable(ctrl->hclk);
+	if (ret)
+		return ret;
+
 	if (ctrl->wake_irq > 0) {
 		if (!irqd_irq_disabled(irq_get_irq_data(ctrl->wake_irq)))
 			disable_irq_nosync(ctrl->wake_irq);
 	}
 
-	clk_prepare_enable(ctrl->hclk);
-
 	if (ctrl->clock_stop_not_supported) {
 		reinit_completion(&ctrl->enumeration);
 		ctrl->reg_write(ctrl, SWRM_COMP_SW_RESET, 0x01);
-- 
2.56.0.317.g454f8302e1-goog

[PATCH 10/21] soundwire: qcom: Serialise ctrl->intr_mask updates

[Lee Jones]

qcom_swrm_irq_handler(), swrm_runtime_suspend(), and
swrm_runtime_resume() concurrently read, modify, and write
ctrl->intr_mask and program the interrupt enable registers without
synchronisation.  Overlapping read-modify-write updates can clobber
interrupt mask bits and leave the hardware interrupt mask out of sync.

Add ctrl->intr_lock to serialise accesses to ctrl->intr_mask and its
associated interrupt mask register updates.

Fixes: 74e79da9fd46 ("soundwire: qcom: add runtime pm support")
Closes: https://sashiko.dev/#/bug/linux-ea523727-6467-42aa-850f-8f838e4b84a8
Cc: stable@vger.kernel.org
Signed-off-by: Lee Jones <lee@kernel.org>
---
 drivers/soundwire/qcom.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
index 555045b32a25..cfa3466ff8e1 100644
--- a/drivers/soundwire/qcom.c
+++ b/drivers/soundwire/qcom.c
@@ -202,6 +202,8 @@ struct qcom_swrm_ctrl {
 	struct completion enumeration;
 	/* Port alloc/free lock */
 	struct mutex port_lock;
+	/* Interrupt mask and broadcast completion lock */
+	struct mutex intr_lock;
 	struct clk *hclk;
 	int irq;
 	u32 ee;
@@ -784,7 +786,9 @@ static irqreturn_t qcom_swrm_irq_handler(int irq, void *dev_id)
 
 	ctrl->reg_read(ctrl, ctrl->reg_layout[SWRM_REG_INTERRUPT_STATUS],
 		       &intr_sts);
+	mutex_lock(&ctrl->intr_lock);
 	intr_sts_masked = intr_sts & ctrl->intr_mask;
+	mutex_unlock(&ctrl->intr_lock);
 
 	do {
 		for (i = 0; i < SWRM_INTERRUPT_MAX; i++) {
@@ -820,10 +824,12 @@ static irqreturn_t qcom_swrm_irq_handler(int irq, void *dev_id)
 				dev_err_ratelimited(ctrl->dev,
 						"%s: SWR bus clsh detected\n",
 						__func__);
+				mutex_lock(&ctrl->intr_lock);
 				ctrl->intr_mask &= ~SWRM_INTERRUPT_STATUS_MASTER_CLASH_DET;
 				ctrl->reg_write(ctrl,
 						ctrl->reg_layout[SWRM_REG_INTERRUPT_CPU_EN],
 						ctrl->intr_mask);
+				mutex_unlock(&ctrl->intr_lock);
 				break;
 			case SWRM_INTERRUPT_STATUS_RD_FIFO_OVERFLOW:
 				ctrl->reg_read(ctrl,
@@ -863,20 +869,24 @@ static irqreturn_t qcom_swrm_irq_handler(int irq, void *dev_id)
 				dev_err_ratelimited(ctrl->dev,
 						"%s: SWR Port collision detected\n",
 						__func__);
+				mutex_lock(&ctrl->intr_lock);
 				ctrl->intr_mask &= ~SWRM_INTERRUPT_STATUS_DOUT_PORT_COLLISION;
 				ctrl->reg_write(ctrl,
 						ctrl->reg_layout[SWRM_REG_INTERRUPT_CPU_EN],
 						ctrl->intr_mask);
+				mutex_unlock(&ctrl->intr_lock);
 				break;
 			case SWRM_INTERRUPT_STATUS_READ_EN_RD_VALID_MISMATCH:
 				dev_err_ratelimited(ctrl->dev,
 					"%s: SWR read enable valid mismatch\n",
 					__func__);
+				mutex_lock(&ctrl->intr_lock);
 				ctrl->intr_mask &=
 					~SWRM_INTERRUPT_STATUS_READ_EN_RD_VALID_MISMATCH;
 				ctrl->reg_write(ctrl,
 						ctrl->reg_layout[SWRM_REG_INTERRUPT_CPU_EN],
 						ctrl->intr_mask);
+				mutex_unlock(&ctrl->intr_lock);
 				break;
 			case SWRM_INTERRUPT_STATUS_SPECIAL_CMD_ID_FINISHED:
 				complete(&ctrl->broadcast);
@@ -910,7 +920,9 @@ static irqreturn_t qcom_swrm_irq_handler(int irq, void *dev_id)
 				intr_sts);
 		ctrl->reg_read(ctrl, ctrl->reg_layout[SWRM_REG_INTERRUPT_STATUS],
 			       &intr_sts);
+		mutex_lock(&ctrl->intr_lock);
 		intr_sts_masked = intr_sts & ctrl->intr_mask;
+		mutex_unlock(&ctrl->intr_lock);
 	} while (intr_sts_masked);
 
 	clk_disable_unprepare(ctrl->hclk);
@@ -952,11 +964,13 @@ static int qcom_swrm_init(struct qcom_swrm_ctrl *ctrl)
 	/* Enable Auto enumeration */
 	ctrl->reg_write(ctrl, SWRM_ENUMERATOR_CFG_ADDR, 1);
 
+	mutex_lock(&ctrl->intr_lock);
 	ctrl->intr_mask = SWRM_INTERRUPT_STATUS_RMSK;
 	/* Mask soundwire interrupts */
 	if (ctrl->version < SWRM_VERSION_2_0_0)
 		ctrl->reg_write(ctrl, ctrl->reg_layout[SWRM_REG_INTERRUPT_MASK_ADDR],
-				SWRM_INTERRUPT_STATUS_RMSK);
+				ctrl->intr_mask);
+	mutex_unlock(&ctrl->intr_lock);
 
 	/* Configure No pings */
 	ctrl->reg_read(ctrl, SWRM_MCP_CFG_ADDR, &val);
@@ -997,8 +1011,10 @@ static int qcom_swrm_init(struct qcom_swrm_ctrl *ctrl)
 			0xFFFFFFFF);
 
 	/* enable CPU IRQs for the selected EE window */
+	mutex_lock(&ctrl->intr_lock);
 	ctrl->reg_write(ctrl, ctrl->reg_layout[SWRM_REG_INTERRUPT_CPU_EN],
-			SWRM_INTERRUPT_STATUS_RMSK);
+			ctrl->intr_mask);
+	mutex_unlock(&ctrl->intr_lock);
 
 	/* Set IRQ to PULSE */
 	ctrl->reg_write(ctrl, SWRM_COMP_CFG_ADDR,
@@ -1689,6 +1705,7 @@ static int qcom_swrm_probe(struct platform_device *pdev)
 	ctrl->dev = dev;
 	dev_set_drvdata(&pdev->dev, ctrl);
 	mutex_init(&ctrl->port_lock);
+	mutex_init(&ctrl->intr_lock);
 	init_completion(&ctrl->broadcast);
 	init_completion(&ctrl->enumeration);
 
@@ -1854,6 +1871,7 @@ static int __maybe_unused swrm_runtime_resume(struct device *dev)
 		ctrl->reg_write(ctrl, ctrl->reg_layout[SWRM_REG_INTERRUPT_CLEAR],
 			SWRM_INTERRUPT_STATUS_MASTER_CLASH_DET);
 
+		mutex_lock(&ctrl->intr_lock);
 		ctrl->intr_mask |= SWRM_INTERRUPT_STATUS_MASTER_CLASH_DET;
 		if (ctrl->version < SWRM_VERSION_2_0_0)
 			ctrl->reg_write(ctrl,
@@ -1861,6 +1879,7 @@ static int __maybe_unused swrm_runtime_resume(struct device *dev)
 					ctrl->intr_mask);
 		ctrl->reg_write(ctrl, ctrl->reg_layout[SWRM_REG_INTERRUPT_CPU_EN],
 				ctrl->intr_mask);
+		mutex_unlock(&ctrl->intr_lock);
 
 		usleep_range(100, 105);
 		if (!swrm_wait_for_frame_gen_enabled(ctrl))
@@ -1882,6 +1901,7 @@ static int __maybe_unused swrm_runtime_suspend(struct device *dev)
 	swrm_wait_for_wr_fifo_done(ctrl);
 	if (!ctrl->clock_stop_not_supported) {
 		/* Mask bus clash interrupt */
+		mutex_lock(&ctrl->intr_lock);
 		ctrl->intr_mask &= ~SWRM_INTERRUPT_STATUS_MASTER_CLASH_DET;
 		if (ctrl->version < SWRM_VERSION_2_0_0)
 			ctrl->reg_write(ctrl,
@@ -1889,6 +1909,7 @@ static int __maybe_unused swrm_runtime_suspend(struct device *dev)
 					ctrl->intr_mask);
 		ctrl->reg_write(ctrl, ctrl->reg_layout[SWRM_REG_INTERRUPT_CPU_EN],
 				ctrl->intr_mask);
+		mutex_unlock(&ctrl->intr_lock);
 		/* Prepare slaves for clock stop */
 		ret = sdw_bus_prep_clk_stop(&ctrl->bus);
 		if (ret < 0 && ret != -ENODATA) {
-- 
2.56.0.317.g454f8302e1-goog

[PATCH 11/21] soundwire: qcom: Prevent stale broadcast IRQ from completing subsequent command

[Lee Jones]

When wait_for_completion_timeout() expires for a broadcast command in
qcom_swrm_cmd_fifo_wr_cmd(), any late
SWRM_INTERRUPT_STATUS_SPECIAL_CMD_ID_FINISHED interrupt from the
timed-out command can fire concurrently with a subsequent broadcast
command after reinit_completion(&ctrl->broadcast), prematurely
completing the new command.

Clear any pending special command completion interrupt and reinitialise
ctrl->broadcast before issuing a broadcast command under
ctrl->intr_lock, clear the interrupt on timeout, and verify that the
interrupt status bit is still asserted under ctrl->intr_lock before
signalling ctrl->broadcast.

Fixes: ddea6cf7b619 ("soundwire: qcom: update register read/write routine")
Closes: https://sashiko.dev/#/bug/linux-fa1aa3a1-f9c7-4e8a-bdfc-49a559353546
Cc: stable@vger.kernel.org
Signed-off-by: Lee Jones <lee@kernel.org>
---
 drivers/soundwire/qcom.c | 44 +++++++++++++++++++++++++++++++++-------
 1 file changed, 37 insertions(+), 7 deletions(-)

diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
index cfa3466ff8e1..5432d3795cea 100644
--- a/drivers/soundwire/qcom.c
+++ b/drivers/soundwire/qcom.c
@@ -554,11 +554,18 @@ static int qcom_swrm_cmd_fifo_wr_cmd(struct qcom_swrm_ctrl *ctrl, u8 cmd_data,
 	if (swrm_wait_for_wr_fifo_avail(ctrl))
 		return SDW_CMD_FAIL_OTHER;
 
-	if (cmd_id == SWR_BROADCAST_CMD_ID)
+	if (cmd_id == SWR_BROADCAST_CMD_ID) {
+		mutex_lock(&ctrl->intr_lock);
+		ctrl->reg_write(ctrl,
+				ctrl->reg_layout[SWRM_REG_INTERRUPT_CLEAR],
+				SWRM_INTERRUPT_STATUS_SPECIAL_CMD_ID_FINISHED);
 		reinit_completion(&ctrl->broadcast);
-
-	/* Its assumed that write is okay as we do not get any status back */
-	ctrl->reg_write(ctrl, ctrl->reg_layout[SWRM_REG_CMD_FIFO_WR_CMD], val);
+		ctrl->reg_write(ctrl, ctrl->reg_layout[SWRM_REG_CMD_FIFO_WR_CMD], val);
+		mutex_unlock(&ctrl->intr_lock);
+	} else {
+		/* Its assumed that write is okay as we do not get any status back */
+		ctrl->reg_write(ctrl, ctrl->reg_layout[SWRM_REG_CMD_FIFO_WR_CMD], val);
+	}
 
 	if (ctrl->version <= SWRM_VERSION_1_3_0)
 		usleep_range(150, 155);
@@ -571,10 +578,16 @@ static int qcom_swrm_cmd_fifo_wr_cmd(struct qcom_swrm_ctrl *ctrl, u8 cmd_data,
 		 */
 		ret = wait_for_completion_timeout(&ctrl->broadcast,
 						  msecs_to_jiffies(TIMEOUT_MS));
-		if (!ret)
+		if (!ret) {
+			mutex_lock(&ctrl->intr_lock);
+			ctrl->reg_write(ctrl,
+					ctrl->reg_layout[SWRM_REG_INTERRUPT_CLEAR],
+					SWRM_INTERRUPT_STATUS_SPECIAL_CMD_ID_FINISHED);
+			mutex_unlock(&ctrl->intr_lock);
 			ret = SDW_CMD_IGNORED;
-		else
+		} else {
 			ret = SDW_CMD_OK;
+		}
 
 	} else {
 		ret = SDW_CMD_OK;
@@ -773,6 +786,22 @@ static irqreturn_t qcom_swrm_wake_irq_handler(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+static void qcom_swrm_complete_broadcast(struct qcom_swrm_ctrl *ctrl)
+{
+	u32 status;
+
+	guard(mutex)(&ctrl->intr_lock);
+
+	ctrl->reg_read(ctrl, ctrl->reg_layout[SWRM_REG_INTERRUPT_STATUS],
+		       &status);
+	if (status & SWRM_INTERRUPT_STATUS_SPECIAL_CMD_ID_FINISHED) {
+		ctrl->reg_write(ctrl,
+				ctrl->reg_layout[SWRM_REG_INTERRUPT_CLEAR],
+				SWRM_INTERRUPT_STATUS_SPECIAL_CMD_ID_FINISHED);
+		complete(&ctrl->broadcast);
+	}
+}
+
 static irqreturn_t qcom_swrm_irq_handler(int irq, void *dev_id)
 {
 	struct qcom_swrm_ctrl *ctrl = dev_id;
@@ -889,7 +918,8 @@ static irqreturn_t qcom_swrm_irq_handler(int irq, void *dev_id)
 				mutex_unlock(&ctrl->intr_lock);
 				break;
 			case SWRM_INTERRUPT_STATUS_SPECIAL_CMD_ID_FINISHED:
-				complete(&ctrl->broadcast);
+				qcom_swrm_complete_broadcast(ctrl);
+				intr_sts &= ~SWRM_INTERRUPT_STATUS_SPECIAL_CMD_ID_FINISHED;
 				break;
 			case SWRM_INTERRUPT_STATUS_BUS_RESET_FINISHED_V2:
 				break;
-- 
2.56.0.317.g454f8302e1-goog

[PATCH 12/21] soundwire: amd: Check for NULL manager drvdata in sdw_amd_startup()

[Lee Jones]

If amd_sdw_manager_probe() fails or defers when registering a SoundWire
manager platform device, dev_get_drvdata(&ctx->pdev[i]->dev) returns
NULL.  sdw_amd_startup() passes this pointer directly to
amd_sdw_manager_start(), which dereferences it unconditionally.

Return -ENODEV in sdw_amd_startup() if dev_get_drvdata() returns NULL,
matching sdw_amd_get_slave_info(), and clean up the allocated context
and platform devices via sdw_amd_exit() if sdw_amd_startup() fails in
sdw_amd_probe().

Fixes: ed5e8741b8db ("soundwire: amd: refactor amd soundwire manager device node creation")
Closes: https://sashiko.dev/#/bug/linux-6834a8a0-632f-4c73-9174-0fe7e06b5303
Cc: stable@vger.kernel.org
Signed-off-by: Lee Jones <lee@kernel.org>
---
 drivers/soundwire/amd_init.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/soundwire/amd_init.c b/drivers/soundwire/amd_init.c
index 8e419ddfa516..adf410fafb7e 100644
--- a/drivers/soundwire/amd_init.c
+++ b/drivers/soundwire/amd_init.c
@@ -156,6 +156,8 @@ static int sdw_amd_startup(struct sdw_amd_ctx *ctx)
 		if (!(ctx->link_mask & BIT(i)))
 			continue;
 		amd_manager = dev_get_drvdata(&ctx->pdev[i]->dev);
+		if (!amd_manager)
+			return -ENODEV;
 		ret = amd_sdw_manager_start(amd_manager);
 		if (ret)
 			return ret;
@@ -166,11 +168,20 @@ static int sdw_amd_startup(struct sdw_amd_ctx *ctx)
 
 int sdw_amd_probe(struct sdw_amd_res *res, struct sdw_amd_ctx **sdw_ctx)
 {
+	int ret;
+
 	*sdw_ctx = sdw_amd_probe_controller(res);
 	if (!*sdw_ctx)
 		return -ENODEV;
 
-	return sdw_amd_startup(*sdw_ctx);
+	ret = sdw_amd_startup(*sdw_ctx);
+	if (ret) {
+		sdw_amd_exit(*sdw_ctx);
+		*sdw_ctx = NULL;
+		return ret;
+	}
+
+	return 0;
 }
 EXPORT_SYMBOL_NS(sdw_amd_probe, "SOUNDWIRE_AMD_INIT");
 
-- 
2.56.0.317.g454f8302e1-goog

[PATCH 13/21] soundwire: amd: Check ping command response for timeout

[Lee Jones]

When amd_sdw_send_cmd_get_resp() times out, it returns -ETIMEDOUT as a
sign-extended 64-bit value.  amd_sdw_read_and_process_ping_status() and
amd_sdw_read_ping_status() extract peripheral status bits directly from
the returned value without checking for -ETIMEDOUT, interpreting the
error code bits as peripheral status values.

Check for -ETIMEDOUT before extracting peripheral status from the ping
response, matching amd_program_scp_addr() and amd_sdw_fill_msg_resp().

Fixes: d8f48fbdfd9a ("soundwire: amd: Add support for AMD Manager driver")
Closes: https://sashiko.dev/#/bug/linux-c3a53ed5-eb8f-4982-a3b6-db189e009b68
Cc: stable@vger.kernel.org
Signed-off-by: Lee Jones <lee@kernel.org>
---
 drivers/soundwire/amd_manager.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/soundwire/amd_manager.c b/drivers/soundwire/amd_manager.c
index a3316efdf8ac..a8220235a2ae 100644
--- a/drivers/soundwire/amd_manager.c
+++ b/drivers/soundwire/amd_manager.c
@@ -448,6 +448,8 @@ static void amd_sdw_read_and_process_ping_status(struct amd_sdw_manager *amd_man
 	mutex_lock(&amd_manager->bus.msg_lock);
 	response = amd_sdw_send_cmd_get_resp(amd_manager, 0, 0);
 	mutex_unlock(&amd_manager->bus.msg_lock);
+	if (response == -ETIMEDOUT)
+		return;
 	amd_sdw_process_ping_status(response, amd_manager);
 }
 
@@ -458,6 +460,8 @@ static u32 amd_sdw_read_ping_status(struct sdw_bus *bus)
 	u32 slave_stat;
 
 	response = amd_sdw_send_cmd_get_resp(amd_manager, 0, 0);
+	if (response == -ETIMEDOUT)
+		return 0;
 	/* slave status from ping response */
 	slave_stat = FIELD_GET(AMD_SDW_MCP_SLAVE_STAT_0_3, response);
 	slave_stat |= FIELD_GET(AMD_SDW_MCP_SLAVE_STAT_4_11, response) << 8;
-- 
2.56.0.317.g454f8302e1-goog

[PATCH 14/21] soundwire: amd: Preserve lane control bits in amd_sdw_port_enable()

[Lee Jones]

lane_ctrl_ch_en_reg holds both the channel enable mask
(AMD_DPN_CH_EN_CHMASK) and the lane control field
(AMD_DPN_CH_EN_LCTRL) programmed by amd_sdw_transport_params().  When
enable_ch->enable is false, amd_sdw_port_enable() writes 0 to the entire
register instead of clearing only AMD_DPN_CH_EN_CHMASK, wiping out the
configured lane control bits.

Clear only the AMD_DPN_CH_EN_CHMASK field when disabling port channels.

Fixes: d8f48fbdfd9a ("soundwire: amd: Add support for AMD Manager driver")
Closes: https://sashiko.dev/#/bug/linux-9a1be49a-7c40-47d3-9260-b933e590c90b
Cc: stable@vger.kernel.org
Signed-off-by: Lee Jones <lee@kernel.org>
---
 drivers/soundwire/amd_manager.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/soundwire/amd_manager.c b/drivers/soundwire/amd_manager.c
index a8220235a2ae..9dfcdde3f2f0 100644
--- a/drivers/soundwire/amd_manager.c
+++ b/drivers/soundwire/amd_manager.c
@@ -706,11 +706,11 @@ static int amd_sdw_port_enable(struct sdw_bus *bus,
 	 * parameters.
 	 */
 	dpn_ch_enable = readl(amd_manager->mmio + lane_ctrl_ch_en_reg);
-	u32p_replace_bits(&dpn_ch_enable, enable_ch->ch_mask, AMD_DPN_CH_EN_CHMASK);
 	if (enable_ch->enable)
-		writel(dpn_ch_enable, amd_manager->mmio + lane_ctrl_ch_en_reg);
+		u32p_replace_bits(&dpn_ch_enable, enable_ch->ch_mask, AMD_DPN_CH_EN_CHMASK);
 	else
-		writel(0, amd_manager->mmio + lane_ctrl_ch_en_reg);
+		u32p_replace_bits(&dpn_ch_enable, 0, AMD_DPN_CH_EN_CHMASK);
+	writel(dpn_ch_enable, amd_manager->mmio + lane_ctrl_ch_en_reg);
 	return 0;
 }
 
-- 
2.56.0.317.g454f8302e1-goog

[PATCH 15/21] soundwire: amd: Avoid static port offset leak in amd_sdw_compute_params()

[Lee Jones]

amd_sdw_compute_params() tracks the next dynamic port block offset in a
static next_offset[] array initialised with {1} (which also leaves
instance 1 initialised to 0).  Because next_offset[] is static and
never reset when streams are freed or when the driver is unbound and
rebound, the offset grows monotonically across stream lifecycles until
it exceeds max_slots - 1 and permanently fails with -ENOMEM.

Compute next_offset locally in amd_sdw_compute_params() from the active
ports on the bus and clear port_offset_map[dai->id] in
amd_sdw_hw_free().

Fixes: 27ab4f1e4909 ("soundwire: amd: refactor bandwidth calculation logic")
Closes: https://sashiko.dev/#/bug/linux-a0378066-b3cf-426c-bc88-e4d5208965b9
Cc: stable@vger.kernel.org
Signed-off-by: Lee Jones <lee@kernel.org>
---
 drivers/soundwire/amd_manager.c | 48 +++++++++++++++++++++++++++++----
 1 file changed, 43 insertions(+), 5 deletions(-)

diff --git a/drivers/soundwire/amd_manager.c b/drivers/soundwire/amd_manager.c
index 9dfcdde3f2f0..98b09d81f3b8 100644
--- a/drivers/soundwire/amd_manager.c
+++ b/drivers/soundwire/amd_manager.c
@@ -469,6 +469,41 @@ static u32 amd_sdw_read_ping_status(struct sdw_bus *bus)
 	return slave_stat;
 }
 
+static unsigned int amd_sdw_find_free_port_offset(struct sdw_bus *bus,
+						  unsigned int slot_size)
+{
+	struct amd_sdw_manager *amd_manager = to_amd_sdw(bus);
+	struct sdw_master_runtime *m_rt;
+	struct sdw_port_runtime *p_rt;
+	unsigned int next_offset = 1;
+	bool overlap;
+
+	do {
+		overlap = false;
+		list_for_each_entry(m_rt, &bus->m_rt_list, bus_node) {
+			unsigned int other_size = m_rt->stream->params.bps *
+						  m_rt->stream->params.ch_count;
+
+			list_for_each_entry(p_rt, &m_rt->port_list, port_node) {
+				unsigned int other_bo;
+
+				if (p_rt->num >= amd_manager->max_ports)
+					continue;
+				other_bo = amd_manager->port_offset_map[p_rt->num];
+				if (!other_bo)
+					continue;
+				if (next_offset < other_bo + other_size &&
+				    other_bo < next_offset + slot_size) {
+					next_offset = other_bo + other_size;
+					overlap = true;
+				}
+			}
+		}
+	} while (overlap);
+
+	return next_offset;
+}
+
 static int amd_sdw_compute_params(struct sdw_bus *bus, struct sdw_stream_runtime *stream)
 {
 	struct amd_sdw_manager *amd_manager = to_amd_sdw(bus);
@@ -479,8 +514,7 @@ static int amd_sdw_compute_params(struct sdw_bus *bus, struct sdw_stream_runtime
 	int port_bo, hstart, hstop, sample_int;
 	unsigned int rate, bps, channels;
 	unsigned int stream_slot_size, max_slots;
-	static unsigned int next_offset[AMD_SDW_MAX_MANAGER_COUNT] = {1};
-	unsigned int inst_id = amd_manager->instance;
+	unsigned int next_offset;
 
 	port_bo = 0;
 	hstart = 1;
@@ -511,11 +545,13 @@ static int amd_sdw_compute_params(struct sdw_bus *bus, struct sdw_stream_runtime
 				 */
 				if (bus->params.curr_dr_freq == 12000000) {
 					max_slots = bus->params.row * (bus->params.col - 1);
-					if (next_offset[inst_id] + stream_slot_size <=
+					next_offset =
+						amd_sdw_find_free_port_offset(bus,
+									      stream_slot_size);
+					if (next_offset + stream_slot_size <=
 					    (max_slots - 1)) {
 						amd_manager->port_offset_map[p_rt->num] =
-									next_offset[inst_id];
-						next_offset[inst_id] += stream_slot_size;
+									next_offset;
 					} else {
 						dev_err(bus->dev,
 							"No space for port %d\n", p_rt->num);
@@ -824,6 +860,8 @@ static int amd_sdw_hw_free(struct snd_pcm_substream *substream, struct snd_soc_d
 	if (ret < 0)
 		dev_err(dai->dev, "remove manager from stream %s failed: %d\n",
 			dai_runtime->stream->name, ret);
+	if (dai->id >= 0 && dai->id < amd_manager->max_ports)
+		amd_manager->port_offset_map[dai->id] = 0;
 	return ret;
 }
 
-- 
2.56.0.317.g454f8302e1-goog

[PATCH 16/21] soundwire: amd: Initialise and cancel IRQ work around manager lifetime

[Lee Jones]

amd_sdw_manager_probe() publishes amd_manager via dev_set_drvdata()
before initialising amd_sdw_irq_thread and amd_sdw_work, allowing an
early interrupt to schedule uninitialised work.  During removal,
amd_sdw_manager_remove() only cancels amd_sdw_work without first
cancelling amd_sdw_irq_thread, allowing deferred IRQ work to re-queue
amd_sdw_work or dereference amd_manager after it is freed.

Initialise both work items before calling dev_set_drvdata() in
amd_sdw_manager_probe(), and cancel amd_sdw_irq_thread before
amd_sdw_work in amd_sdw_manager_remove().

Fixes: 65f93e4096a0 ("soundwire: amd: add SoundWire manager interrupt handling")
Closes: https://sashiko.dev/#/bug/linux-9890e8dc-4b7d-44d3-bfb0-171dc421e28c
Cc: stable@vger.kernel.org
Signed-off-by: Lee Jones <lee@kernel.org>
---
 drivers/soundwire/amd_manager.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/soundwire/amd_manager.c b/drivers/soundwire/amd_manager.c
index 98b09d81f3b8..2e1038ea0f36 100644
--- a/drivers/soundwire/amd_manager.c
+++ b/drivers/soundwire/amd_manager.c
@@ -1185,9 +1185,9 @@ static int amd_sdw_manager_probe(struct platform_device *pdev)
 		sdw_bus_master_delete(&amd_manager->bus);
 		return ret;
 	}
-	dev_set_drvdata(dev, amd_manager);
 	INIT_WORK(&amd_manager->amd_sdw_irq_thread, amd_sdw_irq_thread);
 	INIT_WORK(&amd_manager->amd_sdw_work, amd_sdw_update_slave_status_work);
+	dev_set_drvdata(dev, amd_manager);
 	return 0;
 }
 
@@ -1197,6 +1197,7 @@ static void amd_sdw_manager_remove(struct platform_device *pdev)
 	int ret;
 
 	pm_runtime_disable(&pdev->dev);
+	cancel_work_sync(&amd_manager->amd_sdw_irq_thread);
 	cancel_work_sync(&amd_manager->amd_sdw_work);
 	amd_disable_sdw_interrupts(amd_manager);
 	sdw_bus_master_delete(&amd_manager->bus);
-- 
2.56.0.317.g454f8302e1-goog

[PATCH 17/21] soundwire: amd: Resume manager before hardware teardown in amd_sdw_manager_remove()

[Lee Jones]

amd_sdw_manager_remove() calls pm_runtime_disable() and immediately
accesses MMIO registers in amd_disable_sdw_interrupts() and
amd_disable_sdw_manager() without waking the device if it is
runtime-suspended.

Call pm_runtime_get_sync() before disabling runtime PM and balance the
usage count with pm_runtime_put_noidle() so the controller is awake
during teardown.

Fixes: 81ff58ff71ad ("soundwire: amd: add runtime pm ops for AMD SoundWire manager driver")
Closes: https://sashiko.dev/#/bug/linux-dad4d803-06a7-473d-829d-66860776f4d6
Cc: stable@vger.kernel.org
Signed-off-by: Lee Jones <lee@kernel.org>
---
 drivers/soundwire/amd_manager.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/soundwire/amd_manager.c b/drivers/soundwire/amd_manager.c
index 2e1038ea0f36..1d8a08bda261 100644
--- a/drivers/soundwire/amd_manager.c
+++ b/drivers/soundwire/amd_manager.c
@@ -1196,7 +1196,9 @@ static void amd_sdw_manager_remove(struct platform_device *pdev)
 	struct amd_sdw_manager *amd_manager = dev_get_drvdata(&pdev->dev);
 	int ret;
 
+	pm_runtime_get_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
+	pm_runtime_put_noidle(&pdev->dev);
 	cancel_work_sync(&amd_manager->amd_sdw_irq_thread);
 	cancel_work_sync(&amd_manager->amd_sdw_work);
 	amd_disable_sdw_interrupts(amd_manager);
-- 
2.56.0.317.g454f8302e1-goog

[PATCH 18/21] soundwire: amd: Synchronise parent IRQ and disable work on manager removal

[Lee Jones]

In the ACP top-half interrupt handlers, amd_manager is retrieved via
dev_get_drvdata() and used to schedule amd_sdw_irq_thread when
non-NULL.  If an interrupt is already in flight on another CPU when
amd_sdw_manager_remove() runs, the handler can call
schedule_work(&amd_manager->amd_sdw_irq_thread) after cancel_work_sync()
returns and after devm frees amd_manager.

Disable SoundWire interrupts, synchronise with the parent PCI device
IRQ, disable both manager work items with disable_work_sync(), and mask
interrupts again in case amd_sdw_update_slave_status_work() was running.

Fixes: f93b697ed98e ("soundwire: amd: cancel pending slave status handling workqueue during remove sequence")
Closes: https://sashiko.dev/#/bug/linux-ab458b2e-f505-4a41-8238-a93ceb5d9c77
Cc: stable@vger.kernel.org
Signed-off-by: Lee Jones <lee@kernel.org>
---
 drivers/soundwire/amd_manager.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/soundwire/amd_manager.c b/drivers/soundwire/amd_manager.c
index 1d8a08bda261..f39f4f04db7e 100644
--- a/drivers/soundwire/amd_manager.c
+++ b/drivers/soundwire/amd_manager.c
@@ -8,10 +8,12 @@
 #include <linux/completion.h>
 #include <linux/cleanup.h>
 #include <linux/device.h>
+#include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/jiffies.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/pci.h>
 #include <linux/slab.h>
 #include <linux/soundwire/sdw.h>
 #include <linux/soundwire/sdw_registers.h>
@@ -1199,8 +1201,11 @@ static void amd_sdw_manager_remove(struct platform_device *pdev)
 	pm_runtime_get_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 	pm_runtime_put_noidle(&pdev->dev);
-	cancel_work_sync(&amd_manager->amd_sdw_irq_thread);
-	cancel_work_sync(&amd_manager->amd_sdw_work);
+	amd_disable_sdw_interrupts(amd_manager);
+	if (pdev->dev.parent && dev_is_pci(pdev->dev.parent))
+		synchronize_irq(to_pci_dev(pdev->dev.parent)->irq);
+	disable_work_sync(&amd_manager->amd_sdw_irq_thread);
+	disable_work_sync(&amd_manager->amd_sdw_work);
 	amd_disable_sdw_interrupts(amd_manager);
 	sdw_bus_master_delete(&amd_manager->bus);
 	ret = amd_disable_sdw_manager(amd_manager);
-- 
2.56.0.317.g454f8302e1-goog

[PATCH 19/21] soundwire: amd: Cancel pending work before suspending manager

[Lee Jones]

amd_suspend_runtime() stops the SoundWire bus clock and transitions the
controller into D3 without cancelling amd_sdw_irq_thread and
amd_sdw_work, and amd_suspend() only cancels amd_sdw_work without first
cancelling amd_sdw_irq_thread.  Queued work executing concurrently with
suspend can access MMIO registers after the controller is clock-stopped
or powered down.

Mask manager interrupts, synchronise the parent PCI IRQ, and cancel both
amd_sdw_irq_thread and amd_sdw_work before stopping the bus clock and in
amd_deinit_sdw_manager(), unmask wake interrupts only after clock stop
completes, and avoid non-wake register accesses in amd_sdw_irq_thread()
while the clock is stopped.

Fixes: 81ff58ff71ad ("soundwire: amd: add runtime pm ops for AMD SoundWire manager driver")
Closes: https://sashiko.dev/#/bug/linux-3b774ace-00cf-41e6-82eb-85cd43f5e1d6
Cc: stable@vger.kernel.org
Signed-off-by: Lee Jones <lee@kernel.org>
---
 drivers/soundwire/amd_manager.c | 40 ++++++++++++++++++++++++++++-----
 1 file changed, 34 insertions(+), 6 deletions(-)

diff --git a/drivers/soundwire/amd_manager.c b/drivers/soundwire/amd_manager.c
index f39f4f04db7e..ba8e6d42c467 100644
--- a/drivers/soundwire/amd_manager.c
+++ b/drivers/soundwire/amd_manager.c
@@ -148,9 +148,24 @@ static void amd_disable_sdw_interrupts(struct amd_sdw_manager *amd_manager)
 	writel(0x00, amd_manager->mmio + ACP_SW_ERROR_INTR_MASK);
 }
 
+static void amd_sdw_quiesce_work(struct amd_sdw_manager *amd_manager)
+{
+	struct device *parent = amd_manager->dev->parent;
+
+	disable_work_sync(&amd_manager->amd_sdw_work);
+	writel(0, amd_manager->mmio + ACP_SW_STATE_CHANGE_STATUS_MASK_0TO7);
+	writel(0, amd_manager->mmio + ACP_SW_STATE_CHANGE_STATUS_MASK_8TO11);
+	writel(0, amd_manager->mmio + ACP_SW_ERROR_INTR_MASK);
+	if (parent && dev_is_pci(parent))
+		synchronize_irq(to_pci_dev(parent)->irq);
+	cancel_work_sync(&amd_manager->amd_sdw_irq_thread);
+	enable_work(&amd_manager->amd_sdw_work);
+}
+
 static int amd_deinit_sdw_manager(struct amd_sdw_manager *amd_manager)
 {
 	amd_disable_sdw_interrupts(amd_manager);
+	amd_sdw_quiesce_work(amd_manager);
 	return amd_disable_sdw_manager(amd_manager);
 }
 
@@ -1049,6 +1064,14 @@ static void amd_sdw_irq_thread(struct work_struct *work)
 	u32 status_change_8to11;
 	u32 status_change_0to7;
 
+	if (amd_manager->clk_stopped) {
+		status_change_8to11 = readl(amd_manager->mmio +
+					    ACP_SW_STATE_CHANGE_STATUS_8TO11);
+		if (status_change_8to11 & AMD_SDW_WAKE_STAT_MASK)
+			amd_sdw_process_wake_event(amd_manager);
+		return;
+	}
+
 	status_change_8to11 = readl(amd_manager->mmio + ACP_SW_STATE_CHANGE_STATUS_8TO11);
 	status_change_0to7 = readl(amd_manager->mmio + ACP_SW_STATE_CHANGE_STATUS_0TO7);
 	if (!status_change_0to7 && !status_change_8to11)
@@ -1341,24 +1364,24 @@ static int __maybe_unused amd_suspend(struct device *dev)
 	}
 
 	if (amd_manager->power_mode_mask & AMD_SDW_CLK_STOP_MODE) {
-		cancel_work_sync(&amd_manager->amd_sdw_work);
-		amd_sdw_wake_enable(amd_manager, false);
 		if (amd_manager->acp_rev >= ACP70_PCI_REV_ID) {
 			ret = amd_sdw_host_wake_enable(amd_manager, false);
 			if (ret)
 				return ret;
 		}
+		amd_sdw_quiesce_work(amd_manager);
+		amd_sdw_wake_enable(amd_manager, false);
 		ret = amd_sdw_clock_stop(amd_manager);
 		if (ret)
 			return ret;
 	} else if (amd_manager->power_mode_mask & AMD_SDW_POWER_OFF_MODE) {
-		cancel_work_sync(&amd_manager->amd_sdw_work);
-		amd_sdw_wake_enable(amd_manager, false);
 		if (amd_manager->acp_rev >= ACP70_PCI_REV_ID) {
 			ret = amd_sdw_host_wake_enable(amd_manager, false);
 			if (ret)
 				return ret;
 		}
+		amd_sdw_quiesce_work(amd_manager);
+		amd_sdw_wake_enable(amd_manager, false);
 		/*
 		 * As per hardware programming sequence on AMD platforms,
 		 * clock stop should be invoked first before powering-off
@@ -1391,25 +1414,29 @@ static int __maybe_unused amd_suspend_runtime(struct device *dev)
 		return 0;
 	}
 	if (amd_manager->power_mode_mask & AMD_SDW_CLK_STOP_MODE) {
-		amd_sdw_wake_enable(amd_manager, true);
 		if (amd_manager->acp_rev >= ACP70_PCI_REV_ID) {
 			ret = amd_sdw_host_wake_enable(amd_manager, true);
 			if (ret)
 				return ret;
 		}
+		amd_sdw_quiesce_work(amd_manager);
 		ret = amd_sdw_clock_stop(amd_manager);
 		if (ret)
 			return ret;
+		if (amd_manager->clk_stopped)
+			amd_sdw_wake_enable(amd_manager, true);
 	} else if (amd_manager->power_mode_mask & AMD_SDW_POWER_OFF_MODE) {
-		amd_sdw_wake_enable(amd_manager, true);
 		if (amd_manager->acp_rev >= ACP70_PCI_REV_ID) {
 			ret = amd_sdw_host_wake_enable(amd_manager, true);
 			if (ret)
 				return ret;
 		}
+		amd_sdw_quiesce_work(amd_manager);
 		ret = amd_sdw_clock_stop(amd_manager);
 		if (ret)
 			return ret;
+		if (amd_manager->clk_stopped)
+			amd_sdw_wake_enable(amd_manager, true);
 		ret = amd_deinit_sdw_manager(amd_manager);
 		if (ret)
 			return ret;
@@ -1452,6 +1479,7 @@ static int __maybe_unused amd_resume_runtime(struct device *dev)
 			if (ret)
 				return ret;
 		}
+		amd_enable_sdw_interrupts(amd_manager);
 	} else if (amd_manager->power_mode_mask & AMD_SDW_POWER_OFF_MODE) {
 		writel(0x00, amd_manager->acp_mmio + ACP_SW_WAKE_EN(amd_manager->instance));
 		if (amd_manager->acp_rev >= ACP70_PCI_REV_ID) {
-- 
2.56.0.317.g454f8302e1-goog

[PATCH 20/21] soundwire: amd: Check clock resume timeout in amd_resume_runtime()

[Lee Jones]

When resuming from AMD_SDW_POWER_OFF_MODE in amd_resume_runtime(), the
return value of readl_poll_timeout() when polling for
AMD_SDW_CLK_RESUME_DONE is stored in ret but never checked, allowing
resume to proceed even when clock resume times out.

Return ret if polling for AMD_SDW_CLK_RESUME_DONE fails.

Fixes: 81ff58ff71ad ("soundwire: amd: add runtime pm ops for AMD SoundWire manager driver")
Closes: https://sashiko.dev/#/bug/linux-c6360b30-11b9-40a1-ab04-528462af313c
Cc: stable@vger.kernel.org
Signed-off-by: Lee Jones <lee@kernel.org>
---
 drivers/soundwire/amd_manager.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/soundwire/amd_manager.c b/drivers/soundwire/amd_manager.c
index ba8e6d42c467..faff3e9eefad 100644
--- a/drivers/soundwire/amd_manager.c
+++ b/drivers/soundwire/amd_manager.c
@@ -1494,10 +1494,10 @@ static int __maybe_unused amd_resume_runtime(struct device *dev)
 			ret = readl_poll_timeout(amd_manager->mmio + ACP_SW_CLK_RESUME_CTRL, val,
 						 (val & AMD_SDW_CLK_RESUME_DONE), ACP_DELAY_US,
 						 AMD_SDW_TIMEOUT);
-			if (val & AMD_SDW_CLK_RESUME_DONE) {
-				writel(0, amd_manager->mmio + ACP_SW_CLK_RESUME_CTRL);
-				amd_manager->clk_stopped = false;
-			}
+			if (ret)
+				return ret;
+			writel(0, amd_manager->mmio + ACP_SW_CLK_RESUME_CTRL);
+			amd_manager->clk_stopped = false;
 		}
 		sdw_clear_slave_status(bus, SDW_UNATTACH_REQUEST_MASTER_RESET);
 		ret = amd_sdw_clk_init_ctrl(amd_manager);
-- 
2.56.0.317.g454f8302e1-goog

[PATCH 21/21] soundwire: amd: Disable interrupts if enabling manager fails

[Lee Jones]

In amd_sdw_manager_start() and amd_resume_runtime(),
amd_enable_sdw_interrupts() is called before amd_enable_sdw_manager().
If amd_enable_sdw_manager() fails, both functions return immediately
with SoundWire interrupts left unmasked.  In addition,
amd_resume_runtime() ignores the return value of
amd_init_sdw_manager().

Check the return value of amd_init_sdw_manager() in
amd_resume_runtime(), and disable interrupts if amd_enable_sdw_manager()
fails in amd_sdw_manager_start() and amd_resume_runtime().

Fixes: 81ff58ff71ad ("soundwire: amd: add runtime pm ops for AMD SoundWire manager driver")
Closes: https://sashiko.dev/#/bug/linux-309820e5-f754-4d3d-80aa-7f70d59f3e8e
Cc: stable@vger.kernel.org
Signed-off-by: Lee Jones <lee@kernel.org>
---
 drivers/soundwire/amd_manager.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/soundwire/amd_manager.c b/drivers/soundwire/amd_manager.c
index faff3e9eefad..f64e8fea7e16 100644
--- a/drivers/soundwire/amd_manager.c
+++ b/drivers/soundwire/amd_manager.c
@@ -1109,8 +1109,10 @@ int amd_sdw_manager_start(struct amd_sdw_manager *amd_manager)
 			return ret;
 		amd_enable_sdw_interrupts(amd_manager);
 		ret = amd_enable_sdw_manager(amd_manager);
-		if (ret)
+		if (ret) {
+			amd_disable_sdw_interrupts(amd_manager);
 			return ret;
+		}
 		amd_sdw_set_frameshape(amd_manager);
 	}
 	/* Enable runtime PM */
@@ -1503,11 +1505,15 @@ static int __maybe_unused amd_resume_runtime(struct device *dev)
 		ret = amd_sdw_clk_init_ctrl(amd_manager);
 		if (ret)
 			return ret;
-		amd_init_sdw_manager(amd_manager);
+		ret = amd_init_sdw_manager(amd_manager);
+		if (ret)
+			return ret;
 		amd_enable_sdw_interrupts(amd_manager);
 		ret = amd_enable_sdw_manager(amd_manager);
-		if (ret)
+		if (ret) {
+			amd_disable_sdw_interrupts(amd_manager);
 			return ret;
+		}
 		amd_sdw_set_frameshape(amd_manager);
 	}
 	if (amd_manager->acp_rev >= ACP70_PCI_REV_ID) {
-- 
2.56.0.317.g454f8302e1-goog

Re: [PATCH 03/21] soundwire: stream: Prevent m_rt_count underflow in sdw_stream_remove_master()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] soundwire: stream: m_rt_count leak in sdw_stream_remove_master()
--

commit 029ba42fadc93c4fc5794332114bf86183041b53
Author: Lee Jones <lee@kernel.org>

soundwire: stream: Prevent m_rt_count underflow in sdw_stream_remove_master()

This commit frees partially allocated master ports on allocation failure in
sdw_master_port_alloc() and gates the stream->m_rt_count decrement in
sdw_stream_remove_master() on sdw_master_port_allocated(m_rt).

> diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
> index 0b00fcdcab2d6..f225e5438ccfe 100644
> --- a/drivers/soundwire/stream.c
> +++ b/drivers/soundwire/stream.c

[ ... ]

> @@ -2108,9 +2110,10 @@ int sdw_stream_remove_master(struct sdw_bus *bus,
>  		if (m_rt->bus != bus)
>  			continue;
>  
> +		if (sdw_master_port_allocated(m_rt))
> +			stream->m_rt_count--;
>  		sdw_master_port_free(m_rt);
>  		sdw_master_rt_free(m_rt, stream);
> -		stream->m_rt_count--;

[Severity: Medium]
Can this leave stream->m_rt_count elevated when sdw_stream_add_master() is
called with num_ports == 0?

In sdw_stream_add_master(), when !sdw_master_port_allocated(m_rt) is true,
sdw_master_port_alloc(m_rt, num_ports) is called:

drivers/soundwire/stream.c:sdw_stream_add_master() {
	...
	if (!sdw_master_port_allocated(m_rt)) {
		ret = sdw_master_port_alloc(m_rt, num_ports);
		if (ret)
			goto alloc_error;

		stream->m_rt_count++;
	}
	...
}

When num_ports == 0 (for example via qcom_swrm_hw_params() ->
qcom_swrm_stream_alloc_ports() -> sdw_stream_add_master() when a codec DAI has
no active ports enabled), sdw_master_port_alloc() executes zero loop
iterations and returns 0 while leaving m_rt->port_list empty, and
sdw_stream_add_master() increments stream->m_rt_count.

When sdw_stream_remove_master() later tears down the master runtime,
sdw_master_port_allocated(m_rt) checks !list_empty(&m_rt->port_list), which is
false because m_rt->port_list is empty. As a result, stream->m_rt_count is not
decremented before m_rt is freed, causing subsequent sdw_stream_add_master()
calls on single-link buses (!bus->multi_link) to fail with -EINVAL or
miscounting active master runtimes in do_bank_switch().

Could sdw_master_runtime track whether sdw_stream_add_master() incremented
m_rt_count, or should the num_ports == 0 case be handled consistently between
sdw_stream_add_master() and sdw_stream_remove_master()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030104840.3361217-1-lee@kernel.org?part=3

Re: [PATCH 05/21] ASoC: SOF: Intel: hda-sdw-bpt: Stop active DMA before closing BPT streams

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 542 bytes --]

On Fri, Oct 30, 2026 at 10:46:44AM +0000, Lee Jones wrote:
> When sdw_enable_stream() fails in intel_ace2x_bpt_send_async() after
> hda_sdw_bpt_send_async() has already started the TX and RX DMA streams,
> intel_ace2x_bpt_close_stream() invokes hda_sdw_bpt_close() directly
> without calling hda_sdw_bpt_wait().  As a result,
> hda_sdw_bpt_dma_deprepare() tears down the DMA buffers and BDL while
> the DMA hardware is still running.

Does this actually need to be in with all the Soundwire stuff?  There's
no immediately obvious dependency.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 06/21] soundwire: intel_ace2x: Protect bus.bpt_stream with bus_lock

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] soundwire: mix goto and scoped_guard() in intel_ace2x_bpt_open_stream()
--

commit c2ba69ef3a3456c129416a89de7531b53eb520ca
Author: Lee Jones <lee@kernel.org>

soundwire: intel_ace2x: Protect bus.bpt_stream with bus_lock

This commit protects checks, assignments, and clearing of cdns->bus.bpt_stream
with cdns->bus.bus_lock in intel_ace2x_bpt_open_stream() and
intel_ace2x_bpt_close_stream(), and ensures both master and slave stream
runtimes are removed on open failure.

> diff --git a/drivers/soundwire/intel_ace2x.c b/drivers/soundwire/intel_ace2x.c
> index 642b33ab55260..3623fc9f4451f 100644
> --- a/drivers/soundwire/intel_ace2x.c
> +++ b/drivers/soundwire/intel_ace2x.c
> @@ -83,16 +83,18 @@ static int intel_ace2x_bpt_open_stream(struct sdw_intel *sdw, struct sdw_slave *
>  	int len;
>  	int i;
>  
> -	if (cdns->bus.bpt_stream) {
> -		dev_err(cdns->dev, "%s: BPT stream already exists\n", __func__);
> -		return -EAGAIN;
> -	}
> +	scoped_guard(mutex, &cdns->bus.bus_lock) {
> +		if (cdns->bus.bpt_stream) {
> +			dev_err(cdns->dev, "%s: BPT stream already exists\n", __func__);
> +			return -EAGAIN;
> +		}
>  
> -	stream = sdw_alloc_stream("BPT", SDW_STREAM_BPT);
> -	if (!stream)
> -		return -ENOMEM;
> +		stream = sdw_alloc_stream("BPT", SDW_STREAM_BPT);
> +		if (!stream)
> +			return -ENOMEM;
>  
> -	cdns->bus.bpt_stream = stream;
> +		cdns->bus.bpt_stream = stream;
> +	}
>  
>  	ret = sdw_slave_bpt_stream_add(slave, stream);
>  	if (ret < 0)

[ ... ]

> @@ -293,15 +295,16 @@ static int intel_ace2x_bpt_open_stream(struct sdw_intel *sdw, struct sdw_slave *
>  		dev_err(cdns->dev, "%s: remove master failed: %d\n",
>  			__func__, ret1);
>  
> -remove_slave:
>  	ret1 = sdw_stream_remove_slave(slave, cdns->bus.bpt_stream);
>  	if (ret1 < 0)
>  		dev_err(cdns->dev, "%s: remove slave failed: %d\n",
>  			__func__, ret1);
>  
>  release_stream:
> -	sdw_release_stream(cdns->bus.bpt_stream);
> -	cdns->bus.bpt_stream = NULL;
> +	scoped_guard(mutex, &cdns->bus.bus_lock) {
> +		sdw_release_stream(cdns->bus.bpt_stream);
> +		cdns->bus.bpt_stream = NULL;
> +	}

[Severity: Low]
This isn't a bug, but intel_ace2x_bpt_open_stream() relies on multi-label goto
error unwinding (goto release_stream, goto remove_master, and goto
deprepare_stream) while this patch introduces scoped_guard(mutex,
&cdns->bus.bus_lock) blocks at the start of the function and under the
release_stream label.

Since kernel cleanup API conventions documented in include/linux/cleanup.h
discourage mixing goto-based error handling with scope-based cleanup helpers
in the same function, would it be better to use explicit mutex_lock() and
mutex_unlock() on cdns->bus.bus_lock here instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030104840.3361217-1-lee@kernel.org?part=6

Re: [PATCH 13/21] soundwire: amd: Check ping command response for timeout

[Mukunda,Vijendar]

On 10/30/26 16:16, Lee Jones wrote:
> [You don't often get email from lee@kernel.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> When amd_sdw_send_cmd_get_resp() times out, it returns -ETIMEDOUT as a
> sign-extended 64-bit value.  amd_sdw_read_and_process_ping_status() and
> amd_sdw_read_ping_status() extract peripheral status bits directly from
> the returned value without checking for -ETIMEDOUT, interpreting the
> error code bits as peripheral status values.
>
> Check for -ETIMEDOUT before extracting peripheral status from the ping
> response, matching amd_program_scp_addr() and amd_sdw_fill_msg_resp().
>
> Fixes: d8f48fbdfd9a ("soundwire: amd: Add support for AMD Manager driver")
> Closes: https://sashiko.dev/#/bug/linux-c3a53ed5-eb8f-4982-a3b6-db189e009b68
> Cc: stable@vger.kernel.org
> Signed-off-by: Lee Jones <lee@kernel.org>
> ---
>   drivers/soundwire/amd_manager.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/drivers/soundwire/amd_manager.c b/drivers/soundwire/amd_manager.c
> index a3316efdf8ac..a8220235a2ae 100644
> --- a/drivers/soundwire/amd_manager.c
> +++ b/drivers/soundwire/amd_manager.c
> @@ -448,6 +448,8 @@ static void amd_sdw_read_and_process_ping_status(struct amd_sdw_manager *amd_man
>          mutex_lock(&amd_manager->bus.msg_lock);
>          response = amd_sdw_send_cmd_get_resp(amd_manager, 0, 0);
>          mutex_unlock(&amd_manager->bus.msg_lock);
> +       if (response == -ETIMEDOUT)
> +               return;
++

Response should be u64.
This patch can be dropped.
Please refer below fix.
https://lore.kernel.org/alsa-devel/20261013085111.2009061-6-Vijendar.Mukunda@amd.com/T/#m786cce22308c97d055566dce17be5a38b5d98e9e
I am going to post V4 patch set.


>          amd_sdw_process_ping_status(response, amd_manager);
>   }
>
> @@ -458,6 +460,8 @@ static u32 amd_sdw_read_ping_status(struct sdw_bus *bus)
>          u32 slave_stat;
>
>          response = amd_sdw_send_cmd_get_resp(amd_manager, 0, 0);
> +       if (response == -ETIMEDOUT)
> +               return 0;
>          /* slave status from ping response */
>          slave_stat = FIELD_GET(AMD_SDW_MCP_SLAVE_STAT_0_3, response);
>          slave_stat |= FIELD_GET(AMD_SDW_MCP_SLAVE_STAT_4_11, response) << 8;
> --
> 2.56.0.317.g454f8302e1-goog

Re: [PATCH 05/21] ASoC: SOF: Intel: hda-sdw-bpt: Stop active DMA before closing BPT streams

[Vinod Koul]

On 30-10-26, 11:14, Mark Brown wrote:
> On Fri, Oct 30, 2026 at 10:46:44AM +0000, Lee Jones wrote:
> > When sdw_enable_stream() fails in intel_ace2x_bpt_send_async() after
> > hda_sdw_bpt_send_async() has already started the TX and RX DMA streams,
> > intel_ace2x_bpt_close_stream() invokes hda_sdw_bpt_close() directly
> > without calling hda_sdw_bpt_wait().  As a result,
> > hda_sdw_bpt_dma_deprepare() tears down the DMA buffers and BDL while
> > the DMA hardware is still running.
> 
> Does this actually need to be in with all the Soundwire stuff?  There's
> no immediately obvious dependency.

Doesnt seem dependent to me

-- 
~Vinod

Re: [PATCH 07/21] soundwire: qcom: Prevent pconfig buffer overflow in qcom_swrm_stream_alloc_ports()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] soundwire: qcom: port_mask leak on error in qcom_swrm_stream_alloc_ports()
--

commit 67cffb71aa319b54456dfa761fc2fc2da4b71496
Author: Lee Jones <lee@kernel.org>

soundwire: qcom: Prevent pconfig buffer overflow in qcom_swrm_stream_alloc_ports()

This commit adds a bounds check in qcom_swrm_stream_alloc_ports() to verify
that nports is less than maxport before writing to pconfig[nports], preventing
an out-of-bounds write when more slave ports are requested than ctrl->nports.

> diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
> index 6e0c5fb10efad..2050112340302 100644
> --- a/drivers/soundwire/qcom.c
> +++ b/drivers/soundwire/qcom.c
> @@ -1343,6 +1343,11 @@ static int qcom_swrm_stream_alloc_ports(struct qcom_swrm_ctrl *ctrl,
>  		list_for_each_entry(s_rt, &m_rt->slave_rt_list, m_rt_node) {
>  			slave = s_rt->slave;
>  			list_for_each_entry(p_rt, &s_rt->port_list, port_node) {
> +				if (nports >= maxport) {
> +					dev_err(ctrl->dev, "All ports busy\n");
> +					return -EBUSY;
> +				}

[Severity: High]
Does returning -EBUSY here leak the port allocations in ctrl->port_mask?

In qcom_swrm_stream_alloc_ports(), each loop iteration over s_rt->port_list
sets bit pn in ctrl->port_mask and records pn in the local pconfig array:

	set_bit(pn, port_mask);
	pconfig[nports].num = pn;
	pconfig[nports].ch_mask = p_rt->ch_mask;
	nports++;

When nports reaches maxport on a subsequent iteration and returns -EBUSY (or
when the pn >= maxport check below triggers after earlier iterations already
succeeded), qcom_swrm_stream_alloc_ports() returns before calling
sdw_stream_add_master().

Although the caller qcom_swrm_hw_params() invokes
qcom_swrm_stream_free_ports() on error:

	ret = qcom_swrm_stream_alloc_ports(ctrl, sruntime, params,
					   substream->stream);
	if (ret)
		qcom_swrm_stream_free_ports(ctrl, sruntime);

qcom_swrm_stream_free_ports() only clears bits in ctrl->port_mask by iterating
over m_rt->port_list:

	list_for_each_entry(m_rt, &stream->master_list, stream_node) {
		port_mask = &ctrl->port_mask;
		list_for_each_entry(p_rt, &m_rt->port_list, port_node)
			clear_bit(p_rt->num, port_mask);
	}

Because m_rt->port_list is only populated inside sdw_stream_add_master() and
pconfig is freed on return, m_rt->port_list is still empty when
qcom_swrm_stream_alloc_ports() fails, leaving all previously set bits in
ctrl->port_mask permanently set.

Should qcom_swrm_stream_alloc_ports() unwind the bits set in port_mask from
pconfig[0 .. nports - 1] before returning -EBUSY?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030104840.3361217-1-lee@kernel.org?part=7

Re: [PATCH 14/21] soundwire: amd: Preserve lane control bits in amd_sdw_port_enable()

[Mukunda,Vijendar]

On 10/30/26 16:16, Lee Jones wrote:
> [You don't often get email from lee@kernel.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> lane_ctrl_ch_en_reg holds both the channel enable mask
> (AMD_DPN_CH_EN_CHMASK) and the lane control field
> (AMD_DPN_CH_EN_LCTRL) programmed by amd_sdw_transport_params().  When
> enable_ch->enable is false, amd_sdw_port_enable() writes 0 to the entire
> register instead of clearing only AMD_DPN_CH_EN_CHMASK, wiping out the
> configured lane control bits.
>
> Clear only the AMD_DPN_CH_EN_CHMASK field when disabling port channels.

Multi-lane is not supported on any of the platforms handled by this
function (ACP6.3, ACP7.0, ACP7.1, ACP7.2). AMD_DPN_CH_EN_LCTRL
(bits [2:0]) is never programmed for these platforms, so it always
reads back as 0. Writing 0 to the entire register is therefore
functionally identical to clearing only AMD_DPN_CH_EN_CHMASK.

Upcoming AMD platforms use an entirely different implementation and
will not use this code path either.

The described bug has no practical impact on any hardware this code
runs on, so the Fixes: tag and Cc: stable@vger.kernel.org are not
warranted. Please drop this patch.

>
> Fixes: d8f48fbdfd9a ("soundwire: amd: Add support for AMD Manager driver")
> Closes: https://sashiko.dev/#/bug/linux-9a1be49a-7c40-47d3-9260-b933e590c90b
> Cc: stable@vger.kernel.org
> Signed-off-by: Lee Jones <lee@kernel.org>
> ---
>   drivers/soundwire/amd_manager.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/soundwire/amd_manager.c b/drivers/soundwire/amd_manager.c
> index a8220235a2ae..9dfcdde3f2f0 100644
> --- a/drivers/soundwire/amd_manager.c
> +++ b/drivers/soundwire/amd_manager.c
> @@ -706,11 +706,11 @@ static int amd_sdw_port_enable(struct sdw_bus *bus,
>           * parameters.
>           */
>          dpn_ch_enable = readl(amd_manager->mmio + lane_ctrl_ch_en_reg);
> -       u32p_replace_bits(&dpn_ch_enable, enable_ch->ch_mask, AMD_DPN_CH_EN_CHMASK);
>          if (enable_ch->enable)
> -               writel(dpn_ch_enable, amd_manager->mmio + lane_ctrl_ch_en_reg);
> +               u32p_replace_bits(&dpn_ch_enable, enable_ch->ch_mask, AMD_DPN_CH_EN_CHMASK);
>          else
> -               writel(0, amd_manager->mmio + lane_ctrl_ch_en_reg);
> +               u32p_replace_bits(&dpn_ch_enable, 0, AMD_DPN_CH_EN_CHMASK);
> +       writel(dpn_ch_enable, amd_manager->mmio + lane_ctrl_ch_en_reg);
>          return 0;
>   }
>
> --
> 2.56.0.317.g454f8302e1-goog