From a16f68d3f02026d763482e9cf33501363481b4e4 Mon Sep 17 00:00:00 2001 From: Richard Fitzgerald Date: Fri, 21 Aug 2026 14:58:20 +0100 Subject: [PATCH 1/5] ASoC: Rename snd_soc_dai_link_ch_map.ch_mask to cpu_ch_mask Rename the ch_mask member of snd_soc_dai_link_ch_map to cpu_ch_mask, as that is what it is used for. The CPU and codec channel masks are not necessarily the same, and are quite likely different. SoundWire and I2S/TDM both support assigning different sample slots to each codec, so for example channel 0 on each codec could map to different channels at the CPU. So it's quite normal that the channel mask at the CPU end is different for each codec, but the codec channel masks are the same for each codec. Change-Id: Ia40e1f924320b1426ecbf4e04d3e2d8dc9e4a901 Signed-off-by: Richard Fitzgerald --- include/sound/soc.h | 2 +- sound/soc/sdw_utils/soc_sdw_utils.c | 2 +- sound/soc/soc-pcm.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/include/sound/soc.h b/include/sound/soc.h index f46b2bc2a022bf..94c9b75e27e37c 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -699,7 +699,7 @@ struct snd_soc_dai_link_component { struct snd_soc_dai_link_ch_map { unsigned int cpu; unsigned int codec; - unsigned int ch_mask; + unsigned int cpu_ch_mask; }; struct snd_soc_dai_link { diff --git a/sound/soc/sdw_utils/soc_sdw_utils.c b/sound/soc/sdw_utils/soc_sdw_utils.c index 8a07ba2a29e5a7..221d9c23f684fb 100644 --- a/sound/soc/sdw_utils/soc_sdw_utils.c +++ b/sound/soc/sdw_utils/soc_sdw_utils.c @@ -1570,7 +1570,7 @@ int asoc_sdw_hw_params(struct snd_pcm_substream *substream, * ASoC will set the corresponding channel numbers for each cpu dai. */ for_each_link_ch_maps(rtd->dai_link, i, ch_maps) - ch_maps->ch_mask = ch_mask << (i * step); + ch_maps->cpu_ch_mask = ch_mask << (i * step); return 0; } diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index ae6aa2b1aa4653..ee5a263cbbb6cf 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -1264,7 +1264,7 @@ static int __soc_pcm_hw_params(struct snd_pcm_substream *substream, */ for_each_rtd_ch_maps(rtd, j, ch_maps) if (ch_maps->cpu == i) - ch_mask |= ch_maps->ch_mask; + ch_mask |= ch_maps->cpu_ch_mask; /* fixup cpu channel number */ if (ch_mask) From 4ed7ff783fc0db2ad150cf99246e30f34c7274c3 Mon Sep 17 00:00:00 2001 From: Richard Fitzgerald Date: Fri, 21 Aug 2026 15:04:51 +0100 Subject: [PATCH 2/5] ASoC: Add codec_ch_mask to snd_soc_dai_link_ch_map Add a codec_ch_mask member to snd_soc_dai_link_ch_map. The CPU and codec channel masks are not necessarily the same, and are quite likely different. SoundWire and I2S/TDM both support assigning different sample slots to each codec, so for example channel 0 on each codec could map to different channels at the CPU. So it's quite normal that the channel mask at the CPU end is different for each codec, but the codec channel masks are the same for each codec. Change-Id: Ie2b30fc2dab17a5b2062634fc6bc6439bf83f8e6 Signed-off-by: Richard Fitzgerald --- include/sound/soc.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/sound/soc.h b/include/sound/soc.h index 94c9b75e27e37c..5afc34b147b517 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -700,6 +700,7 @@ struct snd_soc_dai_link_ch_map { unsigned int cpu; unsigned int codec; unsigned int cpu_ch_mask; + unsigned int codec_ch_mask; }; struct snd_soc_dai_link { From 411c332ef4239ed3c15a25cecd9b8536dd4df22c Mon Sep 17 00:00:00 2001 From: Richard Fitzgerald Date: Fri, 21 Aug 2026 15:26:47 +0100 Subject: [PATCH 3/5] ASoC: soc-pcm: Apply snd_soc_dai_link_ch_map.codec_ch_mask to codec params In __soc_pcm_hw_params() if there is a snd_soc_dai_link_ch_map with non-zero codec_ch_mask, use that channel mask to restrict which channels are enabled on the codec. But only if there isn't a TDM mask. It is possible that a snd_soc_dai_link_ch_map could include the same codec multiple times on different CPUs so the for_each_rtd_ch_maps() loop accumulates the channel masks for all entries of that codec. If a TDM mask was also set, it takes priority and is used instead of any possible snd_soc_dai_link_ch_map entries. (They cannot be ANDed together because the bit positions are indicating different things: TDM is a bit for each TDM slot, codec_ch_mask is a bit for each codec channel.) This fixes a problem of incorrect TX channels enabled on the codec when multiple codecs are aggregated on a single capture link. For example: - Two CPUs with six 4-channel codecs. - The machine driver chooses to assign one channel from each codec to one channel on the CPU - But the codec hw_params() would be passed a channel count of 6, which (a) is more channels than the codec has and (b) allows enabling channels that should not be driving the audio bus. Change-Id: I54fbdbe6768bec2cb2f2b50e922ac477b7317d90 Signed-off-by: Richard Fitzgerald --- sound/soc/soc-pcm.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index ee5a263cbbb6cf..da30d0f2901ead 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -1207,6 +1207,9 @@ static int __soc_pcm_hw_params(struct snd_pcm_substream *substream, for_each_rtd_codec_dais(rtd, i, codec_dai) { unsigned int tdm_mask = snd_soc_dai_tdm_mask_get(codec_dai, substream->stream); + struct snd_soc_dai_link_ch_map *ch_maps; + unsigned int ch_mask = 0; + int j; /* * Skip CODECs which don't support the current stream type, @@ -1228,9 +1231,17 @@ static int __soc_pcm_hw_params(struct snd_pcm_substream *substream, /* copy params for each codec */ tmp_params = *params; - /* fixup params based on TDM slot masks */ - if (tdm_mask) + /* fixup params based on TDM or ch_map masks */ + if (tdm_mask) { soc_pcm_codec_params_fixup(&tmp_params, tdm_mask); + } else { + for_each_rtd_ch_maps(rtd, j, ch_maps) + if (ch_maps->codec == i) + ch_mask |= ch_maps->codec_ch_mask; + + if (ch_mask) + soc_pcm_codec_params_fixup(&tmp_params, ch_mask); + } ret = snd_soc_dai_hw_params(codec_dai, substream, &tmp_params); From cce5525e7ea8a357a467abc9fc70a982012c816c Mon Sep 17 00:00:00 2001 From: Richard Fitzgerald Date: Fri, 21 Aug 2026 15:54:06 +0100 Subject: [PATCH 4/5] ASoC: sdw_utils: Set snd_soc_dai_link_ch_map.codec_ch_mask for capture In asoc_sdw_hw_params() set the codec_ch_mask member of struct snd_soc_dai_link_ch_map for capture streams. ASoC will then pass the correct number of channels to each codec hw_params(). This prevents trying to enable more channels on the codec DP than have been allocated bitslots in the SoundWire frame, which would cause bus clash errors. In theory codec_ch_mask could also be set for playback streams, but for those the CPU is the only sender so there is no risk of bus clash. For playback streams codec_ch_mask is set to 0 to preserve the existing behavior and avoid introducing bugs. Change-Id: I0b6e362abda9d91c654140c2470546bc9b6a62b6 Signed-off-by: Richard Fitzgerald --- sound/soc/sdw_utils/soc_sdw_utils.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/sound/soc/sdw_utils/soc_sdw_utils.c b/sound/soc/sdw_utils/soc_sdw_utils.c index 221d9c23f684fb..11a19b98bb1325 100644 --- a/sound/soc/sdw_utils/soc_sdw_utils.c +++ b/sound/soc/sdw_utils/soc_sdw_utils.c @@ -1539,7 +1539,7 @@ int asoc_sdw_hw_params(struct snd_pcm_substream *substream, struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream); struct snd_soc_dai_link_ch_map *ch_maps; int ch = params_channels(params); - unsigned int ch_mask; + unsigned int cpu_ch_mask, codec_ch_mask; int num_codecs; int step; int i; @@ -1549,8 +1549,9 @@ int asoc_sdw_hw_params(struct snd_pcm_substream *substream, /* Identical data will be sent to all codecs in playback */ if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { - ch_mask = GENMASK(ch - 1, 0); + cpu_ch_mask = GENMASK(ch - 1, 0); step = 0; + codec_ch_mask = 0; } else { num_codecs = rtd->dai_link->num_codecs; @@ -1560,17 +1561,24 @@ int asoc_sdw_hw_params(struct snd_pcm_substream *substream, return -EINVAL; } - ch_mask = GENMASK(ch / num_codecs - 1, 0); - step = hweight_long(ch_mask); + cpu_ch_mask = GENMASK(ch / num_codecs - 1, 0); + step = hweight_long(cpu_ch_mask); + codec_ch_mask = cpu_ch_mask; } /* * The captured data will be combined from each cpu DAI if the dai * link has more than one codec DAIs. Set codec channel mask and * ASoC will set the corresponding channel numbers for each cpu dai. + * + * sdw_stream_add_slave() assigns different payload offsets to each + * codec in a capture stream, so that the same channels on each + * codec map to different channels on the CPU. */ - for_each_link_ch_maps(rtd->dai_link, i, ch_maps) - ch_maps->cpu_ch_mask = ch_mask << (i * step); + for_each_link_ch_maps(rtd->dai_link, i, ch_maps) { + ch_maps->cpu_ch_mask = cpu_ch_mask << (i * step); + ch_maps->codec_ch_mask = codec_ch_mask; + } return 0; } From 5e400492e77fa9db4da3e4209d73fd230e8dcc48 Mon Sep 17 00:00:00 2001 From: Richard Fitzgerald Date: Fri, 21 Aug 2026 16:24:43 +0100 Subject: [PATCH 5/5] ASoC: sdw_utils: cs_amp: Delete bogus and incorrect capture channel fixup Delete the asoc_sdw_cs_spk_feedback_rtd_init(). This is not needed now that the ASoC bug it was working around has been fixed. And it was broken anyway, creating a confusing mapping of amp channels to capture channels. This code was added to avoid a problem where multiple codec DP outputs were mapped to the same SoundWire frame bit slot. This would allow a user to break the SoundWire bus just by enabling mixer outputs using ALSA controls. As no production system has used the capture stream, this workaround was of little consequence and the problem of conflicting DP mappings was not investigated. The ASoC bug that enabled too many channels on each codec has now been fixed. So this workaround can be completely deleted. Change-Id: Ia4d193bf7db33179277d23a09090178918d2848a Signed-off-by: Richard Fitzgerald --- include/sound/soc_sdw_utils.h | 2 -- sound/soc/sdw_utils/soc_sdw_cs_amp.c | 46 ---------------------------- sound/soc/sdw_utils/soc_sdw_utils.c | 4 --- 3 files changed, 52 deletions(-) diff --git a/include/sound/soc_sdw_utils.h b/include/sound/soc_sdw_utils.h index 9b28e9aef4f1d6..9fbb69b9052df4 100644 --- a/include/sound/soc_sdw_utils.h +++ b/include/sound/soc_sdw_utils.h @@ -250,8 +250,6 @@ int asoc_sdw_cs_amp_init(struct snd_soc_card *card, struct snd_soc_dai_link *dai_links, struct asoc_sdw_codec_info *info, bool playback); -int asoc_sdw_cs_spk_feedback_rtd_init(struct snd_soc_pcm_runtime *rtd, - struct snd_soc_dai *dai); int asoc_sdw_cs35l56_volume_limit(struct snd_soc_card *card, const char *name_prefix); /* MAXIM codec support */ diff --git a/sound/soc/sdw_utils/soc_sdw_cs_amp.c b/sound/soc/sdw_utils/soc_sdw_cs_amp.c index 325ab723048172..6e21ef8f87e27c 100644 --- a/sound/soc/sdw_utils/soc_sdw_cs_amp.c +++ b/sound/soc/sdw_utils/soc_sdw_cs_amp.c @@ -14,7 +14,6 @@ #include #include -#define CS_AMP_CHANNELS_PER_AMP 4 #define CS35L56_SPK_VOLUME_0DB 400 /* 0dB Max */ int asoc_sdw_cs35l56_volume_limit(struct snd_soc_card *card, const char *name_prefix) @@ -64,51 +63,6 @@ int asoc_sdw_cs_spk_rtd_init(struct snd_soc_pcm_runtime *rtd, struct snd_soc_dai } EXPORT_SYMBOL_NS(asoc_sdw_cs_spk_rtd_init, "SND_SOC_SDW_UTILS"); -int asoc_sdw_cs_spk_feedback_rtd_init(struct snd_soc_pcm_runtime *rtd, struct snd_soc_dai *dai) -{ - const struct snd_soc_dai_link *dai_link = rtd->dai_link; - const struct snd_soc_dai_link_ch_map *ch_map; - const struct snd_soc_dai_link_component *codec_dlc; - struct snd_soc_dai *codec_dai; - u8 ch_slot[8] = {}; - unsigned int amps_per_bus, ch_per_amp, mask; - int i, ret; - - WARN_ON(dai_link->num_cpus > ARRAY_SIZE(ch_slot)); - - /* - * CS35L56 has 4 TX channels. When the capture is aggregated the - * same bus slots will be allocated to all the amps on a bus. Only - * one amp on that bus can be transmitting in each slot so divide - * the available 4 slots between all the amps on a bus. - */ - amps_per_bus = dai_link->num_codecs / dai_link->num_cpus; - if ((amps_per_bus == 0) || (amps_per_bus > CS_AMP_CHANNELS_PER_AMP)) { - dev_err(rtd->card->dev, "Illegal num_codecs:%u / num_cpus:%u\n", - dai_link->num_codecs, dai_link->num_cpus); - return -EINVAL; - } - - ch_per_amp = CS_AMP_CHANNELS_PER_AMP / amps_per_bus; - - for_each_rtd_ch_maps(rtd, i, ch_map) { - codec_dlc = snd_soc_link_to_codec(rtd->dai_link, i); - codec_dai = snd_soc_find_dai(codec_dlc); - mask = GENMASK(ch_per_amp - 1, 0) << ch_slot[ch_map->cpu]; - - ret = snd_soc_dai_set_tdm_slot(codec_dai, 0, mask, 4, 32); - if (ret < 0) { - dev_err(rtd->card->dev, "Failed to set TDM slot:%d\n", ret); - return ret; - } - - ch_slot[ch_map->cpu] += ch_per_amp; - } - - return 0; -} -EXPORT_SYMBOL_NS(asoc_sdw_cs_spk_feedback_rtd_init, "SND_SOC_SDW_UTILS"); - int asoc_sdw_cs_amp_init(struct snd_soc_card *card, struct snd_soc_dai_link *dai_links, struct asoc_sdw_codec_info *info, diff --git a/sound/soc/sdw_utils/soc_sdw_utils.c b/sound/soc/sdw_utils/soc_sdw_utils.c index 11a19b98bb1325..089469e664d1e1 100644 --- a/sound/soc/sdw_utils/soc_sdw_utils.c +++ b/sound/soc/sdw_utils/soc_sdw_utils.c @@ -800,7 +800,6 @@ struct asoc_sdw_codec_info codec_info_list[] = { .dai_name = "cs35l56-sdw1c", .dai_type = SOC_SDW_DAI_TYPE_AMP, .dailink = {SOC_SDW_UNUSED_DAI_ID, SOC_SDW_AMP_IN_DAI_ID}, - .rtd_init = asoc_sdw_cs_spk_feedback_rtd_init, }, }, .dai_num = 2, @@ -829,7 +828,6 @@ struct asoc_sdw_codec_info codec_info_list[] = { .dai_name = "cs35l56-sdw1c", .dai_type = SOC_SDW_DAI_TYPE_AMP, .dailink = {SOC_SDW_UNUSED_DAI_ID, SOC_SDW_AMP_IN_DAI_ID}, - .rtd_init = asoc_sdw_cs_spk_feedback_rtd_init, }, }, .dai_num = 2, @@ -858,7 +856,6 @@ struct asoc_sdw_codec_info codec_info_list[] = { .dai_name = "cs35l56-sdw1c", .dai_type = SOC_SDW_DAI_TYPE_AMP, .dailink = {SOC_SDW_UNUSED_DAI_ID, SOC_SDW_AMP_IN_DAI_ID}, - .rtd_init = asoc_sdw_cs_spk_feedback_rtd_init, }, }, .dai_num = 2, @@ -887,7 +884,6 @@ struct asoc_sdw_codec_info codec_info_list[] = { .dai_name = "cs35l56-sdw1c", .dai_type = SOC_SDW_DAI_TYPE_AMP, .dailink = {SOC_SDW_UNUSED_DAI_ID, SOC_SDW_AMP_IN_DAI_ID}, - .rtd_init = asoc_sdw_cs_spk_feedback_rtd_init, }, }, .dai_num = 2,