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

[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

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

[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

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

[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

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

[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

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

[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

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

[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

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 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 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 16/21] soundwire: amd: Initialise and cancel IRQ work around manager lifetime

[Lee Jones]

On Fri, 30 Oct 2026, Mario Limonciello wrote:
> 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?

I will leave the AMD driver alone.

Please be kind enough to add my Reported-by for any issued that you fix
as a result of these reports.

Thanks.

-- 
Lee Jones

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

[Mukunda,Vijendar]

On 11/2/26 15:01, Lee Jones wrote:
> [Some people who received this message don't often get email from lee@kernel.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> On Fri, 30 Oct 2026, Mario Limonciello wrote:
>> 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?
> I will leave the AMD driver alone.
>
> Please be kind enough to add my Reported-by for any issued that you fix
> as a result of these reports.
Sure, thanks for reporting these issues.

I'll make sure to include your Reported-by: tag for any fixes resulting
from these reports.
>
> Thanks.
>
> --
> Lee Jones

[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

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

[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

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

[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

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

[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

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 20/21] soundwire: amd: Check clock resume timeout in amd_resume_runtime()

[Mukunda,Vijendar]

On 10/30/26 18:25, Mukunda,Vijendar wrote:
>
>
> 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.
Correction to my earlier reply.
After further review, the existing timeout handling is intentional.
Please disregard my earlier statement that the timeout should be
propagated.

A similar CLK_RESUME_DONE timeout issue was discussed earlier:
https://lore.kernel.org/all/fbaae15c-f3f7-4125-96aa-41b6bb4f174b@amd.com/#t

As discussed there, propagating -ETIMEDOUT would abort the PM resume
path rather than allowing the remaining recovery sequence to proceed.
That discussion also noted the same timeout-handling pattern in the
POWER_OFF_MODE path.
For POWER_OFF_MODE, returning an error at this point would skip the
subsequent slave-state reset and SoundWire manager re-initialization,
preventing the bus recovery and codec re-enumeration.
The preferred long-term approach is to recover the bus on such failures
rather than propagate the error to the PM framework. This remains a
TODO.
Therefore, we intend to retain the existing timeout behavior and will
not change it in the upcoming respin.
Please drop this patch. Sorry for the confusion caused by my earlier
reply.
>>>
>>> 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);
>>
>

[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 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 21/21] soundwire: amd: Disable interrupts if enabling manager fails

[Mukunda,Vijendar]

On 10/30/26 18:28, Mukunda,Vijendar wrote:
>
>
> 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.
Correction to the above. After a more detailed analysis of the ACP
hardware interrupt routing, the fix is not required. Please drop
this change and disregard our earlier statement.

The AMD ACP IP is the parent device that dispatches SoundWire
interrupts per manager instance. It only forwards an SDW interrupt
to the CPU when the corresponding SoundWire manager is active.
If acp_enable_sdw_manager() times out the ACP treats that manager
instance as inactive and will not assert any SDW interrupt on its
behalf, regardless of the mask register state.

Two independent hardware guarantees prevent any interrupt from
firing after a manager-enable failure:

1. The masks written by acp_enable_sdw_interrupts() cover
    packet-based SoundWire status events. These require the
    SoundWire clock to be running, which in turn requires the
    manager to be enabled. Without an active manager there is
    no clock, no SoundWire packet can be received, and no
    interrupt is generated.

2. Wake interrupt masks are only functional once the SoundWire
    manager is enabled in the D0 sequence. Additionally, wake
    is explicitly disabled at the very start of the POWER_OFF_MODE
    resume path, well before acp_enable_sdw_manager() is reached:

      writel(0x00, acp_mmio + ACP_SW_WAKE_EN(instance));
      amd_sdw_host_wake_enable_by_rev(amd_manager, false);
      ...
      acp_enable_sdw_interrupts(amd_manager);
      ret = acp_enable_sdw_manager(amd_manager);  /* may fail here */

v5 will not include a fix for this case.
>>
>> 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) {
>