[PATCH v2 0/2] soundwire: qcom: Restore and fix set_channel_map support for SWR master

[PATCH v2 0/2] soundwire: qcom: Restore and fix set_channel_map support for SWR master

[Mohammad Rafi Shaik]

The original support for set_channel_map in the Qualcomm SoundWire master
commit 7796c97df6b1 ("soundwire: qcom: Add set_channel_map api support")
was reverted due to multiple issues reported on sdm845 board, including
kernel crashes and data structure corruption. The root causes included
incorrect port indexing, array size mismatches, and improper handling
of TX/RX channel information. This series reintroduces set_channel_map
support with a corrected and safer implementation, and fixes the sdm845
machine driver to avoid misuse ofthe API.

Add a robust qcom_swrm_set_channel_map() implementation that stores
per-port TX/RX channel masks safely under controller lock. Ensure the
configuration is only applied to valid ports and does not corrupt memory
or overwrite unrelated data. Preserve existing behavior for ports that do
not use set_channel_map, by falling back to the default SoundWire stream
configuration. Fix the sdm845 machine driver to avoid applying SLIMbus
channel maps to SoundWire DAIs.

Signed-off-by: Mohammad Rafi Shaik <mohammad.rafi.shaik@oss.qualcomm.com>
---
Changes in v2:
- Removed the set_stream hack from the SDM845 machine driver.
- Link to v1: https://lore.kernel.org/all/20260629141633.86657-1-mohammad.rafi.shaik@oss.qualcomm.com/

To: Srinivas Kandagatla <srini@kernel.org>
To: Liam Girdwood <lgirdwood@gmail.com>
To: Mark Brown <broonie@kernel.org>
To: Jaroslav Kysela <perex@perex.cz>
To: Takashi Iwai <tiwai@suse.com>
To: Vinod Koul <vkoul@kernel.org>
To: Bard Liao <yung-chuan.liao@linux.intel.com>
To: Pierre-Louis Bossart <pierre-louis.bossart@linux.dev>
To: Mohammad Rafi Shaik <quic_mohs@quicinc.com>
cc: Yongqin Liu <yongqin.liu@linaro.org>
cc: Jie Gan <jie.gan@oss.qualcomm.com>
Cc: linux-sound@vger.kernel.org
Cc: linux-arm-msm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

---
Mohammad Rafi Shaik (2):
      ASoC: qcom: sdm845: apply the SLIMbus channel map to wcd934x only
      soundwire: qcom: Add set_channel_map support to SWR master DAI

 drivers/soundwire/qcom.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++
 sound/soc/qcom/sdm845.c  | 16 ++++++++++++++
 2 files changed, 70 insertions(+)
---
base-commit: 8b7179210b22f8e48a82c5ceacb9d56a9c4cfec4
change-id: 20261105-soundwire_channel_map-e3fb482c04cb

Best regards,
--  
Mohammad Rafi Shaik <mohammad.rafi.shaik@oss.qualcomm.com>

[PATCH v2 v2 1/2] ASoC: qcom: sdm845: apply the SLIMbus channel map to wcd934x only

[Mohammad Rafi Shaik]

On sdm845 boards the SLIMbus backends are described in DT with several
codec DAIs on the same DAI link. The SLIM playback link lists the two
WSA881x speakers and the SoundWire master embedded in wcd934x next to
the wcd934x SLIMbus AIF:

	codec {
		sound-dai = <&left_spkr>, <&right_spkr>,
			    <&swm 0>, <&wcd9340 AIF1_PB>;
	};

sdm845_dai_init() walks all codec DAIs of such a link and calls
snd_soc_dai_set_channel_map() on each of them with the hardcoded wcd934x
SLIMbus channel numbers, 128..143 for TX and 144..156 for RX. Those
channel numbers are only meaningful for a codec attached over SLIMbus,
and programming them into the SoundWire master DAI configures bogus port
channel masks on it.

So far this has been harmless only because neither the qcom SoundWire
master nor the WSA881x codecs implement .set_channel_map, so the call
returns -ENOTSUPP and is silently ignored. It starts corrupting the
SoundWire port configuration as soon as the master DAI implements that
op, which the next patch does.

The channel map is not the only wcd934x specific setup in this loop. The
codec MCLK rate (WCD934X_DEFAULT_MCLK_RATE) and the headset jack belong
to wcd934x as well and are broadcast to the SoundWire DAIs for the same
reason, relying on them returning -ENOTSUPP.

Match the wcd934x codec explicitly by its driver name and skip the other
codec DAIs of the link, so that each codec only gets the setup that is
meant for it.

The bus type of the DAI device cannot be used to tell these codecs
apart: wcd934x is a SLIMbus device driven by an MFD driver that creates
the ASoC component and the SoundWire master as platform children of it
(wcd934x_devices[] in drivers/mfd/wcd934x.c), so both DAIs live on the
platform bus with the same SLIMbus parent.

Signed-off-by: Mohammad Rafi Shaik <mohammad.rafi.shaik@oss.qualcomm.com>
---
 sound/soc/qcom/sdm845.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/sound/soc/qcom/sdm845.c b/sound/soc/qcom/sdm845.c
index 85951397cb2b..7137c9c14d8f 100644
--- a/sound/soc/qcom/sdm845.c
+++ b/sound/soc/qcom/sdm845.c
@@ -234,6 +234,19 @@ static void sdm845_jack_free(struct snd_jack *jack)
 	snd_soc_component_set_jack(component, NULL, NULL);
 }
 
+/*
+ * The SLIMbus backends list the wcd934x codec, the SoundWire master
+ * embedded in wcd934x and its SoundWire speaker codecs as codec DAIs of
+ * the same DAI link.  Only the wcd934x DAIs take the SLIMbus channel map
+ * and the codec clock and jack setup below.
+ */
+static bool sdm845_dai_is_wcd934x(struct snd_soc_dai *dai)
+{
+	struct device_driver *drv = dai->dev->driver;
+
+	return drv && !strcmp(drv->name, "wcd934x-codec");
+}
+
 static int sdm845_dai_init(struct snd_soc_pcm_runtime *rtd)
 {
 	struct snd_soc_component *component;
@@ -301,6 +314,9 @@ static int sdm845_dai_init(struct snd_soc_pcm_runtime *rtd)
 			return 0;
 
 		for_each_rtd_codec_dais(rtd, i, codec_dai) {
+			if (!sdm845_dai_is_wcd934x(codec_dai))
+				continue;
+
 			rval = snd_soc_dai_set_channel_map(codec_dai,
 							  ARRAY_SIZE(tx_ch),
 							  tx_ch,

-- 
2.34.1

[PATCH v2 v2 2/2] soundwire: qcom: Add set_channel_map support to SWR master DAI

[Mohammad Rafi Shaik]

Add .set_channel_map support to the SWR master DAI so the ASoC
layer can program per-port channel masks used by port_enable().

Fix several issues in channel map handling: clip programming to
available ports to avoid out-of-bounds access, use 1-based port
indexing, and store TX/RX masks separately. Also track per-direction
validity and select the effective mask based on port role to avoid
incorrect precedence and ensure full-width masks are preserved.
Fall back to the SDW stream-provided channel mask when no mapping is
configured, preserving existing behaviour.

Fixes: 7796c97df6b1 ("soundwire: qcom: Add set_channel_map api support")
Reported-by: Yongqin Liu <yongqin.liu@linaro.org>
Closes: https://lore.kernel.org/all/4ddd6855-3817-4dc4-81c4-d8ddaa039865@oss.qualcomm.com/
Reported-by: Jie Gan <jie.gan@oss.qualcomm.com>
Closes: https://lore.kernel.org/all/CAMSo37U1kJq_gK8jiW9iMbhHXtn=Chr7NawiK4fPUPj4kyqH2w@mail.gmail.com/
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/all/33fe8fe7-719a-405a-9ed2-d9f816ce1d57@sabinyo.mountain/
Signed-off-by: Mohammad Rafi Shaik <mohammad.rafi.shaik@oss.qualcomm.com>
---
 drivers/soundwire/qcom.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
index 147e55b7a600..c411c3628dee 100644
--- a/drivers/soundwire/qcom.c
+++ b/drivers/soundwire/qcom.c
@@ -162,6 +162,10 @@ struct qcom_swrm_port_config {
 	u8 word_length;
 	u8 blk_group_count;
 	u8 lane_control;
+	u8 tx_ch_mask;
+	u8 rx_ch_mask;
+	bool tx_ch_map_valid;
+	bool rx_ch_map_valid;
 };
 
 /*
@@ -1188,12 +1192,28 @@ static int qcom_swrm_port_enable(struct sdw_bus *bus,
 {
 	u32 reg;
 	struct qcom_swrm_ctrl *ctrl = to_qcom_sdw(bus);
+	struct qcom_swrm_port_config *pcfg;
 	u32 val;
 	u32 offset = ctrl->reg_layout[SWRM_OFFSET_DP_PORT_CTRL_BANK];
 
 	reg = SWRM_DPn_PORT_CTRL_BANK(offset, enable_ch->port_num, bank);
 
 	ctrl->reg_read(ctrl, reg, &val);
+	pcfg = &ctrl->pconfig[enable_ch->port_num];
+
+	mutex_lock(&ctrl->port_lock);
+	if (enable_ch->port_num <= ctrl->num_dout_ports) {
+		if (pcfg->rx_ch_map_valid)
+			enable_ch->ch_mask = pcfg->rx_ch_mask;
+		else if (pcfg->tx_ch_map_valid)
+			enable_ch->ch_mask = pcfg->tx_ch_mask;
+	} else {
+		if (pcfg->tx_ch_map_valid)
+			enable_ch->ch_mask = pcfg->tx_ch_mask;
+		else if (pcfg->rx_ch_map_valid)
+			enable_ch->ch_mask = pcfg->rx_ch_mask;
+	}
+	mutex_unlock(&ctrl->port_lock);
 
 	if (enable_ch->enable)
 		val |= (enable_ch->ch_mask << SWRM_DP_PORT_CTRL_EN_CHAN_SHFT);
@@ -1413,6 +1433,39 @@ static void *qcom_swrm_get_sdw_stream(struct snd_soc_dai *dai, int direction)
 	return ctrl->sruntime[dai->id];
 }
 
+static int qcom_swrm_set_channel_map(struct snd_soc_dai *dai,
+				     unsigned int tx_num, const unsigned int *tx_slot,
+				     unsigned int rx_num, const unsigned int *rx_slot)
+{
+	struct qcom_swrm_ctrl *ctrl = dev_get_drvdata(dai->dev);
+	unsigned int max_ports = ctrl->nports;
+	unsigned int i;
+
+	if ((tx_num && !tx_slot) || (rx_num && !rx_slot))
+		return -EINVAL;
+
+	mutex_lock(&ctrl->port_lock);
+	for (i = 1; i <= max_ports; i++) {
+		ctrl->pconfig[i].tx_ch_map_valid = false;
+		ctrl->pconfig[i].rx_ch_map_valid = false;
+
+		/* TX setup */
+		if (tx_slot && i < tx_num) {
+			ctrl->pconfig[i].tx_ch_mask = tx_slot[i];
+			ctrl->pconfig[i].tx_ch_map_valid = true;
+		}
+
+		/* RX setup */
+		if (rx_slot && i < rx_num) {
+			ctrl->pconfig[i].rx_ch_mask = rx_slot[i];
+			ctrl->pconfig[i].rx_ch_map_valid = true;
+		}
+	}
+	mutex_unlock(&ctrl->port_lock);
+
+	return 0;
+}
+
 static int qcom_swrm_startup(struct snd_pcm_substream *substream,
 			     struct snd_soc_dai *dai)
 {
@@ -1449,6 +1502,7 @@ static const struct snd_soc_dai_ops qcom_swrm_pdm_dai_ops = {
 	.shutdown = qcom_swrm_shutdown,
 	.set_stream = qcom_swrm_set_sdw_stream,
 	.get_stream = qcom_swrm_get_sdw_stream,
+	.set_channel_map = qcom_swrm_set_channel_map,
 };
 
 static const struct snd_soc_component_driver qcom_swrm_dai_component = {

-- 
2.34.1

Re: [PATCH v2 v2 2/2] soundwire: qcom: Add set_channel_map support to SWR master DAI

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] soundwire: qcom: lock ordering inversion in qcom_swrm_port_enable()
- [High] soundwire: qcom: zero slot masks in qcom_swrm_set_channel_map() clobber ports
- [High] soundwire: qcom: single-direction qcom_swrm_set_channel_map() clobbers other direction
--

commit c509f8b3c2506c16cc2d00c47ddc8257189902cf
Author: Mohammad Rafi Shaik <mohammad.rafi.shaik@oss.qualcomm.com>

soundwire: qcom: Add set_channel_map support to SWR master DAI

This commit adds .set_channel_map support to the Qualcomm SoundWire master
DAI so the ASoC layer can configure per-port TX and RX channel masks used by
qcom_swrm_port_enable().

> diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
> index 147e55b7a600b..c411c3628dee4 100644
> --- a/drivers/soundwire/qcom.c
> +++ b/drivers/soundwire/qcom.c

[ ... ]

> @@ -1188,12 +1192,28 @@ static int qcom_swrm_port_enable(struct sdw_bus *bus,
>  {
>  	u32 reg;
>  	struct qcom_swrm_ctrl *ctrl = to_qcom_sdw(bus);
> +	struct qcom_swrm_port_config *pcfg;
>  	u32 val;
>  	u32 offset = ctrl->reg_layout[SWRM_OFFSET_DP_PORT_CTRL_BANK];
>  
>  	reg = SWRM_DPn_PORT_CTRL_BANK(offset, enable_ch->port_num, bank);
>  
>  	ctrl->reg_read(ctrl, reg, &val);
> +	pcfg = &ctrl->pconfig[enable_ch->port_num];
> +
> +	mutex_lock(&ctrl->port_lock);

[Severity: High]
Does acquiring ctrl->port_lock inside qcom_swrm_port_enable() cause a lock
ordering inversion with ctrl->bus.bus_lock?

During hw_params, qcom_swrm_hw_params() -> qcom_swrm_stream_alloc_ports()
acquires ctrl->port_lock and then calls sdw_stream_add_master(), which locks
bus->bus_lock:

drivers/soundwire/qcom.c:qcom_swrm_stream_alloc_ports() {
	guard(mutex)(&ctrl->port_lock);
	...
	sdw_stream_add_master(&ctrl->bus, &sconfig, pconfig,
			      nports, stream);
}

During stream enable and disable, sdw_enable_stream() and sdw_disable_stream()
acquire bus->bus_lock via sdw_acquire_bus_lock() before calling into
bus->port_ops->dpn_port_enable_ch(), which invokes qcom_swrm_port_enable() and
acquires ctrl->port_lock in the reverse order (bus->bus_lock ->
ctrl->port_lock).

> +	if (enable_ch->port_num <= ctrl->num_dout_ports) {
> +		if (pcfg->rx_ch_map_valid)
> +			enable_ch->ch_mask = pcfg->rx_ch_mask;
> +		else if (pcfg->tx_ch_map_valid)
> +			enable_ch->ch_mask = pcfg->tx_ch_mask;
> +	} else {
> +		if (pcfg->tx_ch_map_valid)
> +			enable_ch->ch_mask = pcfg->tx_ch_mask;
> +		else if (pcfg->rx_ch_map_valid)
> +			enable_ch->ch_mask = pcfg->rx_ch_mask;
> +	}
> +	mutex_unlock(&ctrl->port_lock);
>  
>  	if (enable_ch->enable)
>  		val |= (enable_ch->ch_mask << SWRM_DP_PORT_CTRL_EN_CHAN_SHFT);
> @@ -1413,6 +1433,39 @@ static void *qcom_swrm_get_sdw_stream(struct snd_soc_dai *dai, int direction)
>  	return ctrl->sruntime[dai->id];
>  }
>  
> +static int qcom_swrm_set_channel_map(struct snd_soc_dai *dai,
> +				     unsigned int tx_num, const unsigned int *tx_slot,
> +				     unsigned int rx_num, const unsigned int *rx_slot)
> +{
> +	struct qcom_swrm_ctrl *ctrl = dev_get_drvdata(dai->dev);
> +	unsigned int max_ports = ctrl->nports;
> +	unsigned int i;
> +
> +	if ((tx_num && !tx_slot) || (rx_num && !rx_slot))
> +		return -EINVAL;
> +
> +	mutex_lock(&ctrl->port_lock);
> +	for (i = 1; i <= max_ports; i++) {
> +		ctrl->pconfig[i].tx_ch_map_valid = false;
> +		ctrl->pconfig[i].rx_ch_map_valid = false;

[Severity: High]
As reported by Sashiko on v1 [1], when qcom_swrm_set_channel_map() is called
for a single stream direction, does unconditionally resetting both
tx_ch_map_valid and rx_ch_map_valid to false clobber the opposite direction's
channel map?

In qcom_snd_sdw_startup(), playback calls snd_soc_dai_set_channel_map() with
tx_num = 0 and rx_num = SDW_MAX_PORTS, whereas capture calls it with
tx_num = SDW_MAX_PORTS and rx_num = 0. Because all DAIs on the controller
share ctrl->pconfig[], starting a playback stream clears tx_ch_map_valid across
all ports and sets rx_ch_map_valid = true, while starting a capture stream
clears rx_ch_map_valid and sets tx_ch_map_valid = true.

In addition, because qcom_swrm_port_enable() falls back to the opposite
direction's mask when a port's own direction flag is false:

	if (enable_ch->port_num <= ctrl->num_dout_ports) {
		if (pcfg->rx_ch_map_valid)
			enable_ch->ch_mask = pcfg->rx_ch_mask;
		else if (pcfg->tx_ch_map_valid)
			enable_ch->ch_mask = pcfg->tx_ch_mask;
	} else {
		if (pcfg->tx_ch_map_valid)
			enable_ch->ch_mask = pcfg->tx_ch_mask;
		else if (pcfg->rx_ch_map_valid)
			enable_ch->ch_mask = pcfg->rx_ch_mask;
	}

won't DIN (TX) ports end up using pcfg->rx_ch_mask after a playback
set_channel_map() call, and DOUT (RX) ports end up using pcfg->tx_ch_mask
after a capture set_channel_map() call?

> +
> +		/* TX setup */
> +		if (tx_slot && i < tx_num) {
> +			ctrl->pconfig[i].tx_ch_mask = tx_slot[i];
> +			ctrl->pconfig[i].tx_ch_map_valid = true;
> +		}
> +
> +		/* RX setup */
> +		if (rx_slot && i < rx_num) {
> +			ctrl->pconfig[i].rx_ch_mask = rx_slot[i];
> +			ctrl->pconfig[i].rx_ch_map_valid = true;
> +		}

[Severity: High]
As reported by Sashiko on v1 [2], if tx_slot[i] or rx_slot[i] is 0 for an
unused or unmapped port in the sparse channel map array, will marking
tx_ch_map_valid or rx_ch_map_valid as true cause qcom_swrm_port_enable() to
override enable_ch->ch_mask with 0?

Codec callbacks such as wcd937x_get_channel_map() and pm4125_get_channel_map()
populate a 15-element (SDW_MAX_PORTS) sparse array where unmapped or inactive
master ports have slot[i] == 0:

sound/soc/codecs/wcd937x.c:wcd937x_get_channel_map() {
	for (i = 0; i < SDW_MAX_PORTS; i++)
		rx_slot[i] = wcd->master_channel_map[i];

	*rx_num = i;
}

Since max_ports <= 14 < SDW_MAX_PORTS, i < tx_num or i < rx_num is true for
every port 1..max_ports. Any port whose slot entry is 0 gets tx_ch_mask = 0 or
rx_ch_mask = 0 with the valid flag set to true, so qcom_swrm_port_enable() will
overwrite the stream-provided enable_ch->ch_mask with 0 instead of falling
back to the stream's channel mask.

[1] https://lore.kernel.org/r/20260629141633.86657-3-mohammad.rafi.shaik@oss.qualcomm.com
[2] https://lore.kernel.org/r/20260629141633.86657-3-mohammad.rafi.shaik@oss.qualcomm.com

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105-soundwire_channel_map-v2-0-5efe2021552a@oss.qualcomm.com?part=2