[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

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

[Lee Jones]

On Fri, 30 Oct 2026, Mukunda,Vijendar wrote:

> 
> 
> 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.

Thanks for the review.

Wilco.  I'll go manually turn down the Sashiko bug to this effect as well.

-- 
Lee Jones

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

[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 ]
>
> 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().

This is not a bug for ACP6.3/7.0/7.1/7.2 platforms. These variants
do not have banked registers for data port programming. Instead, a
fixed port mapping per DAI ID is used by design: each use case (e.g.
headset playback, headphone capture) always maps to the same DAI ID
and port number across stream lifecycles.

The guard at line 402:

     if (!amd_manager->port_offset_map[p_rt->num])

ensures that port_offset_map[] is populated exactly once per port
at first use and never reassigned. Since the DAI-to-port mapping is
fixed, next_offset[] advancing after initial assignment has no effect
on subsequent stream open/close cycles.

Furthermore, the hw_free change introduced by this patch:

     amd_manager->port_offset_map[dai->id] = 0;

would be a regression. Clearing the map entry on stream teardown
causes the port to be reassigned a new block offset on the next
stream open, at which point next_offset[] has already advanced past
the original value. This breaks the fixed mapping invariant that the
driver relies on.

The number of concurrent streams per instance is known and bounded,
so -ENOMEM from slot exhaustion cannot occur in practice.
Please drop this patch.
>
> 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

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

[Lee Jones]

On Fri, 30 Oct 2026, Vinod Koul wrote:

> 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

Sashiko grouped this report under the SoundWire subsystem because the
caller (intel_ace2x_bpt_close_stream()) lives in
drivers/soundwire/intel_ace2x.c, even though hda_sdw_bpt_close() itself
is under sound/soc/sof/intel/.

You're both right, there is no dependency on the rest of the series.

Happy for you to take it directly through ASoC if the patch looks good
to you or I can drop it from v2 of the SoundWire set and send it
standalone or when I handle ASoC.

-- 
Lee Jones

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

[Lee Jones]

On Fri, 30 Oct 2026, Mukunda,Vijendar wrote:

> 
> 
> 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.

Excellent.  Will do.  Thanks again.

-- 
Lee Jones

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

[Lee Jones]

On Fri, 30 Oct 2026, Mukunda,Vijendar wrote:

> 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 ]
> > 
> > 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().
> 
> This is not a bug for ACP6.3/7.0/7.1/7.2 platforms. These variants
> do not have banked registers for data port programming. Instead, a
> fixed port mapping per DAI ID is used by design: each use case (e.g.
> headset playback, headphone capture) always maps to the same DAI ID
> and port number across stream lifecycles.
> 
> The guard at line 402:
> 
>     if (!amd_manager->port_offset_map[p_rt->num])
> 
> ensures that port_offset_map[] is populated exactly once per port
> at first use and never reassigned. Since the DAI-to-port mapping is
> fixed, next_offset[] advancing after initial assignment has no effect
> on subsequent stream open/close cycles.
> 
> Furthermore, the hw_free change introduced by this patch:
> 
>     amd_manager->port_offset_map[dai->id] = 0;
> 
> would be a regression. Clearing the map entry on stream teardown
> causes the port to be reassigned a new block offset on the next
> stream open, at which point next_offset[] has already advanced past
> the original value. This breaks the fixed mapping invariant that the
> driver relies on.
> 
> The number of concurrent streams per instance is known and bounded,
> so -ENOMEM from slot exhaustion cannot occur in practice.
> Please drop this patch.

Done.

-- 
Lee Jones

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

[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 ]
>
> 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);
The probe-side ordering issue is valid — an interrupt arriving after
dev_set_drvdata() but before INIT_WORK() could schedule work on an
uninitialised work_struct.

The remove-side fix is already posted as part of our v3 patch series:

https://lore.kernel.org/alsa-devel/20261013085111.2009061-6-Vijendar.Mukunda@amd.com/T/#m60177687bb84758efdd6f5f09eba8ad32f0e4631

The probe-side fix will be included in our upcoming v4 patch series,
which contains major driver restructuring and customer-critical bug
fixes. Merging this patch independently would conflict with v4 and
complicate the review process.

Could you please drop this patch? Both issues will be covered by our
v3/v4 series.

>          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);
proper fix already posted as part of v3 version.
https://lore.kernel.org/alsa-devel/20261013085111.2009061-6-Vijendar.Mukunda@amd.com/T/#m60177687bb84758efdd6f5f09eba8ad32f0e4631

>          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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] soundwire: amd: stale status[] processed on ping timeout
--

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

soundwire: amd: Check ping command response for timeout

This commit adds checks for -ETIMEDOUT after calling
amd_sdw_send_cmd_get_resp() in amd_sdw_read_and_process_ping_status() and
amd_sdw_read_ping_status() so that a timeout error code is not decoded as
peripheral status bits.

> diff --git a/drivers/soundwire/amd_manager.c b/drivers/soundwire/amd_manager.c
> index a3316efdf8ac2..a8220235a2ae3 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;

[Severity: Medium]
When amd_sdw_send_cmd_get_resp() times out here, returning early without
propagating an error leaves amd_manager->status[] containing stale status
values while callers proceed as if the status array was refreshed.

In drivers/soundwire/amd_manager.c, amd_sdw_update_slave_status_work()
relies on amd_sdw_read_and_process_ping_status() to overwrite
amd_manager->status[] before jumping back to update_status:

	if (amd_manager->status[0] == SDW_SLAVE_ATTACHED) {
		if (retry_count++ < SDW_MAX_DEVICES) {
			writel(AMD_SDW_IRQ_MASK_0TO7, amd_manager->mmio +
			       ACP_SW_STATE_CHANGE_STATUS_MASK_0TO7);
			writel(AMD_SDW_IRQ_MASK_8TO11, amd_manager->mmio +
			       ACP_SW_STATE_CHANGE_STATUS_MASK_8TO11);
			amd_sdw_read_and_process_ping_status(amd_manager);
			goto update_status;
		}
		...
	}

Because sdw_handle_slave_status() only reads amd_manager->status[] and
does not clear status[0], a ping timeout leaves status[0] set to
SDW_SLAVE_ATTACHED (along with any other stale entries in status[]),
causing the loop to repeatedly call sdw_handle_slave_status() with stale
peripheral status up to SDW_MAX_DEVICES times.

Similarly, in amd_sdw_irq_thread():

	if (status_change_8to11 & AMD_SDW_PREQ_INTR_STAT) {
		amd_sdw_read_and_process_ping_status(amd_manager);
	} else {
		/* Check for the updated status on peripheral device */
		amd_sdw_update_slave_status(status_change_0to7, status_change_8to11, amd_manager);
	}
	if (status_change_8to11 || status_change_0to7)
		schedule_work(&amd_manager->amd_sdw_work);

If the ping command times out when handling AMD_SDW_PREQ_INTR_STAT,
amd_sdw_work is still scheduled and will process stale status values left
in amd_manager->status[] from a previous interrupt.

Should amd_sdw_read_and_process_ping_status() return an error code so that
its callers can avoid reprocessing stale amd_manager->status[] entries on
timeout?

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

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

[Lee Jones]

On Fri, 30 Oct 2026, Mukunda,Vijendar wrote:

> 
> 
> 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 ]
> > 
> > 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);
> The probe-side ordering issue is valid — an interrupt arriving after
> dev_set_drvdata() but before INIT_WORK() could schedule work on an
> uninitialised work_struct.
> 
> The remove-side fix is already posted as part of our v3 patch series:
> 
> https://lore.kernel.org/alsa-devel/20261013085111.2009061-6-Vijendar.Mukunda@amd.com/T/#m60177687bb84758efdd6f5f09eba8ad32f0e4631
> 
> The probe-side fix will be included in our upcoming v4 patch series,
> which contains major driver restructuring and customer-critical bug
> fixes. Merging this patch independently would conflict with v4 and
> complicate the review process.
> 
> Could you please drop this patch? Both issues will be covered by our
> v3/v4 series.

Yes, no problem.

-- 
Lee Jones

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

[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 ]
>
> 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);
The SoundWire manager is a child platform device of the ACP PCI
device and does not have a dedicated BAR. Its MMIO is mapped through
the parent ACP PCI device's address space, which remains valid and
accessible for the lifetime of the parent device.

Runtime-suspending the SoundWire manager child device does not unmap
or gate that MMIO region. As long as the parent ACP PCI device is
alive — which it is during child device teardown — MMIO access is
always safe, regardless of the child's runtime PM state.

The described scenario cannot occur on this hardware. Please drop
this patch.
>          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

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

[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 ]
>
> 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);
The SoundWire manager is a child platform device of the ACP PCI
device and does not have a dedicated BAR. Its MMIO is mapped through
the parent ACP PCI device's address space, which remains valid and
accessible for the lifetime of the parent device.

Runtime-suspending the SoundWire manager child device does not unmap
or gate that MMIO region. As long as the parent ACP PCI device is
alive — which it is during child device teardown — MMIO access is
always safe, regardless of the child's runtime PM state.

The described scenario cannot occur on this hardware. Please drop
this patch.
>          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

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

[Péter Ujfalusi]

On 30/10/2026 12:46, 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.
> 
> 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>

Acked-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>

> ---
>  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.

-- 
Péter

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

[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 ]
>
> 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);
++

The synchronize_irq() approach is incorrect for this hardware. The
ACP IP uses a dedicated line-based interrupt. Using
to_pci_dev(pdev->dev.parent)->irq gives the parent PCI device's
primary IRQ, which is not the SoundWire interrupt line. Synchronizing
the wrong IRQ does not ensure the ACP SoundWire top-half handler has
completed on another CPU.

Additionally, disable_work_sync() is not available in older stable
kernels, making the Cc: stable tag unworkable without further
backport effort.

The double call to amd_disable_sdw_interrupts() is also unjustified —
the work handler does not re-enable interrupts.

We have addressed this race in our upcoming v4 patch series using an
interrupt_enabled flag checked in the top-half before scheduling
work. This is the standard pattern for this type of race and has been
validated on the hardware. Please drop this patch.
>          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

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

[Mukunda,Vijendar]

On 10/30/26 16:16, Lee Jones wrote:
> 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.

Both issues are already addressed in our v3 patch series:

https://lore.kernel.org/alsa-devel/20261013085111.2009061-6-Vijendar.Mukunda@amd.com/T/#m7741edcedb1264764edc7f8559e39af8b23433fb

In our v3 implementation, both the suspend and suspend_runtime paths
call acp_disable_sdw_interrupts() followed by cancel_work_sync() for
both amd_sdw_irq_thread and amd_sdw_work, in that order, before
clock stop — for both CLK_STOP_MODE and POWER_OFF_MODE.

Additionally, the amd_sdw_quiesce_work() helper introduced by this
patch has the same synchronize_irq() issue raised on the previous
patch — to_pci_dev(parent)->irq is not the correct IRQ line for the
ACP dedicated interrupt. The disable_work_sync()/enable_work()
pattern also introduces a new window where a late IRQ can re-queue
work after enable_work() but before clock stop completes.

Please drop this patch. The fix is already on-list as part of our
v3 series, and v4 with further improvements is being prepared.
> 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) {

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

[Mario Limonciello]

On 10/30/26 07:13, Mukunda,Vijendar wrote:
> 
> 
> 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 ]
>>
>> 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);
> The probe-side ordering issue is valid — an interrupt arriving after
> dev_set_drvdata() but before INIT_WORK() could schedule work on an
> uninitialised work_struct.
> 
> The remove-side fix is already posted as part of our v3 patch series:
> 
> https://lore.kernel.org/alsa-devel/20261013085111.2009061-6- 
> Vijendar.Mukunda@amd.com/T/#m60177687bb84758efdd6f5f09eba8ad32f0e4631
> 
> The probe-side fix will be included in our upcoming v4 patch series,
> which contains major driver restructuring and customer-critical bug
> fixes. Merging this patch independently would conflict with v4 and
> complicate the review process.
> 
> Could you please drop this patch? Both issues will be covered by our
> v3/v4 series.
> 
Lee,

I think as a general statement since there is still these big pending 
changes already in flight, could you either wait a few weeks until they 
merge or rebase your series on the in-flight version to see what still 
actually makes sense?

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

[Mukunda,Vijendar]

On 10/30/26 16:16, Lee Jones wrote:
> 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;
++

The bug is valid. On clock resume timeout the code falls through to
sdw_clear_slave_status() and full bus re-init without the clock
being active, which is incorrect.

However, we have identified additional issues in this code path that
need to be addressed together:

1. writel(0, ACP_SW_CLK_RESUME_CTRL) is incorrect — this register
    is not a plain zero-clear register. Only the Clk_Resume_Req bit
    should be cleared using a read-modify-write on the success path.

2. The same unchecked ret pattern exists in
    amd_acp63_sdw_clock_stop_exit() and needs to be fixed together.

We are planning to address all of these as part of our upcoming v4
patch series which will be posted shortly. Could you please hold
this patch until v4 is on-list to avoid conflicts?
>   		}
>   		sdw_clear_slave_status(bus, SDW_UNATTACH_REQUEST_MASTER_RESET);
>   		ret = amd_sdw_clk_init_ctrl(amd_manager);

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

[Mukunda,Vijendar]

On 10/30/26 18:21, Mukunda,Vijendar wrote:
>
>
> On 10/30/26 16:16, Lee Jones wrote:
>> 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;
> ++
>
> The bug is valid. On clock resume timeout the code falls through to
> sdw_clear_slave_status() and full bus re-init without the clock
> being active, which is incorrect.
>
> However, we have identified additional issues in this code path that
> need to be addressed together:
>
> 1. writel(0, ACP_SW_CLK_RESUME_CTRL) is incorrect — this register
>    is not a plain zero-clear register. Only the Clk_Resume_Req bit
>    should be cleared using a read-modify-write on the success path.
Slight correction.  Ignore comment 1.

>
> 2. The same unchecked ret pattern exists in
>    amd_acp63_sdw_clock_stop_exit() and needs to be fixed together.
>
> We are planning to address all of these as part of our upcoming v4
> patch series which will be posted shortly. Could you please hold
> this patch until v4 is on-list to avoid conflicts?
>>           }
>>           sdw_clear_slave_status(bus, 
>> SDW_UNATTACH_REQUEST_MASTER_RESET);
>>           ret = amd_sdw_clk_init_ctrl(amd_manager);
>

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

[Mukunda,Vijendar]

On 10/30/26 16:17, Lee Jones wrote:
> 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().
++
The unchecked return value of amd_init_sdw_manager() in
amd_resume_runtime() is already addressed in our v3 patch series:

https://lore.kernel.org/alsa-devel/20261013085111.2009061-6-Vijendar.Mukunda@amd.com/T/#m7741edcedb1264764edc7f8559e39af8b23433fb

The missing amd_disable_sdw_interrupts() on amd_enable_sdw_manager()
failure is a valid observation and will be addressed in our upcoming
v4 patch series. Please hold this patch until v4 is on-list to
avoid conflicts.
>
> 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) {

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

[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 ]
>
> 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().

The sdw_amd_probe() cleanup fix — calling sdw_amd_exit() and
nulling *sdw_ctx on sdw_amd_startup() failure — is already
addressed in our v3 patch series posted for upstream review:

https://lore.kernel.org/alsa-devel/20261013085111.2009061-6-Vijendar.Mukunda@amd.com/T/#u

The NULL check in sdw_amd_startup() is a valid observation and
will be addressed in our upcoming v4 patch series. Please hold
this patch until v4 is on-list to avoid conflicts.

>
> 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

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

[Liao, Bard]

> -----Original Message-----
> From: Lee Jones <lee@kernel.org>
> Sent: Friday, October 30, 2026 6:47 PM
> To: lee@kernel.org; Liam Girdwood <lgirdwood@gmail.com>; Peter Ujfalusi
> <peter.ujfalusi@linux.intel.com>; Bard Liao <yung-chuan.liao@linux.intel.com>;
> Daniel Baluta <daniel.baluta@nxp.com>; Kai Vehmanen
> <kai.vehmanen@linux.intel.com>; Pierre-Louis Bossart <pierre-
> louis.bossart@linux.dev>; Vijendar Mukunda <Vijendar.Mukunda@amd.com>;
> Mark Brown <broonie@kernel.org>; Jaroslav Kysela <perex@perex.cz>; Takashi
> Iwai <tiwai@suse.com>; Vinod Koul <vkoul@kernel.org>; Ranjani Sridharan
> <ranjani.sridharan@linux.intel.com>; sound-open-firmware@alsa-project.org;
> linux-sound@vger.kernel.org; linux-kernel@vger.kernel.org
> Cc: stable@vger.kernel.org
> Subject: [PATCH 05/21] ASoC: SOF: Intel: hda-sdw-bpt: Stop active DMA before
> closing BPT streams
> 
> 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)

bpt_rx_stream and bpt_tx_stream will never be NULL. The current code
doesn't test it, too.

> +		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
> 

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] soundwire: amd: stale clk_stopped flag in amd_sdw_irq_thread() after power-off
- [High] soundwire: amd: runtime PM deadlock in amd_sdw_quiesce_work()
- [Medium] soundwire: amd: wake IRQ unmasked then cancelled in amd_suspend_runtime()
--

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

soundwire: amd: Cancel pending work before suspending manager

This commit masks manager interrupts, synchronizes the parent PCI IRQ, and
cancels both amd_sdw_irq_thread and amd_sdw_work before stopping the SoundWire
bus clock during suspend and manager deinitialization. It also avoids non-wake
register accesses in amd_sdw_irq_thread() while the clock is stopped.

> diff --git a/drivers/soundwire/amd_manager.c b/drivers/soundwire/amd_manager.c
> index f39f4f04db7e9..ba8e6d42c467f 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);

[Severity: High]
Can calling disable_work_sync(&amd_manager->amd_sdw_work) from
amd_suspend_runtime() cause a runtime PM deadlock if a peripheral alert is
being processed concurrently?

In the SoundWire device hierarchy (amd_manager->dev -> bus->dev ->
slave->dev), neither amd_manager->dev nor bus->dev sets power.ignore_children.
When all child slave->dev devices are runtime-suspended, the manager remains
active until its autosuspend delay expires.

When rpm_suspend() runs on amd_manager->dev, its runtime status is set to
RPM_SUSPENDING before invoking:

  amd_suspend_runtime()
    -> amd_sdw_quiesce_work()
      -> disable_work_sync(&amd_manager->amd_sdw_work)

If an alert interrupt scheduled amd_sdw_work just prior to this, the worker
executes:

  amd_sdw_update_slave_status_work()
    -> sdw_handle_slave_status()
      -> sdw_handle_slave_alerts()
        -> pm_runtime_get_sync(&slave->dev)

Because slave->dev and bus->dev are RPM_SUSPENDED, rpm_resume(&slave->dev,
RPM_GET_PUT) synchronously resumes the parent chain up to
rpm_resume(amd_manager->dev, 0), which sees RPM_SUSPENDING and waits in
TASK_UNINTERRUPTIBLE on amd_manager->dev->power.wait_queue for
amd_suspend_runtime() to complete, while amd_suspend_runtime() is waiting in
disable_work_sync() for amd_sdw_update_slave_status_work() to finish.

> +	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;
> +	}

[Severity: High]
Does this early return drop all non-wake interrupts after resuming from
AMD_SDW_POWER_OFF_MODE when the parent ACP controller has been reset?

During amd_suspend() or amd_suspend_runtime() in AMD_SDW_POWER_OFF_MODE,
amd_sdw_clock_stop() sets amd_manager->clk_stopped = true and
amd_deinit_sdw_manager() clears ACP_SW_EN. When the parent ACP PCI device
subsequently suspends and resumes, check_acp_sdw_enable_status() observes
ACP_SW0_EN and ACP_SW1_EN cleared and performs a full ACP soft reset and power
cycle, which resets ACP_SW_CLK_RESUME_CTRL to 0.

When amd_resume_runtime() then runs the AMD_SDW_POWER_OFF_MODE path in
drivers/soundwire/amd_manager.c:

		val = readl(amd_manager->mmio + ACP_SW_CLK_RESUME_CTRL);
		if (val) {
			val |= AMD_SDW_CLK_RESUME_REQ;
			writel(val, amd_manager->mmio + ACP_SW_CLK_RESUME_CTRL);
			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;
			}
		}

Because val is 0 after the ACP reset, amd_manager->clk_stopped is never reset
to false even after amd_sdw_clk_init_ctrl(), amd_init_sdw_manager(),
amd_enable_sdw_interrupts(), and amd_enable_sdw_manager() reinitialize and
restart the bus.

With this new check in amd_sdw_irq_thread(), wouldn't every subsequent
interrupt return early without reading ACP_SW_STATE_CHANGE_STATUS_0TO7,
processing peripheral attachment or ping status, scheduling amd_sdw_work, or
clearing ACP_SW_STATE_CHANGE_STATUS_8TO11 and
ACP_SW_STATE_CHANGE_STATUS_0TO7?

[ ... ]

> @@ -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);

[Severity: Medium]
In the AMD_SDW_POWER_OFF_MODE branch of amd_suspend_runtime(),
amd_sdw_wake_enable(amd_manager, true) unmasks AMD_SDW_WAKE_INTR_MASK in
ACP_SW_STATE_CHANGE_STATUS_MASK_8TO11 while ACP_EXTERNAL_INTR_CNTL still has
the manager interrupt enabled.

Immediately after that, amd_deinit_sdw_manager() is called:

  amd_deinit_sdw_manager()
    -> amd_disable_sdw_interrupts()
    -> amd_sdw_quiesce_work()
      -> cancel_work_sync(&amd_manager->amd_sdw_irq_thread)

amd_disable_sdw_interrupts() zeroes ACP_SW_STATE_CHANGE_STATUS_MASK_8TO11 and
clears the manager interrupt bit in ACP_EXTERNAL_INTR_CNTL, and
amd_sdw_quiesce_work() cancels amd_sdw_irq_thread.

If a wake interrupt fires in the window between amd_sdw_wake_enable() and
amd_disable_sdw_interrupts(), the top-half IRQ handler acknowledges the
interrupt in ACP_EXTERNAL_INTR_STAT and schedules amd_sdw_irq_thread, which is
then cancelled by amd_deinit_sdw_manager() before amd_sdw_process_wake_event()
can call pm_request_resume(). Why unmask AMD_SDW_WAKE_INTR_MASK here if
amd_deinit_sdw_manager() immediately masks it and cancels the work?

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