[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <33fe8fe7-719a-405a-9ed2-d9f816ce1d57@sabinyo.mountain>
Date: Fri, 27 Jun 2025 15:59:04 -0500
From: Dan Carpenter <dan.carpenter@...aro.org>
To: Mohammad Rafi Shaik <quic_mohs@...cinc.com>
Cc: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
Liam Girdwood <lgirdwood@...il.com>,
Mark Brown <broonie@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Vinod Koul <vkoul@...nel.org>,
Bard Liao <yung-chuan.liao@...ux.intel.com>,
Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>,
Pierre-Louis Bossart <pierre-louis.bossart@...ux.dev>,
Sanyog Kale <sanyog.r.kale@...el.com>,
linux-arm-msm@...r.kernel.org, linux-sound@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
quic_pkumpatl@...cinc.com, kernel@....qualcomm.com
Subject: Re: [PATCH v6 3/4] soundwire: qcom: Add set_channel_map api support
On Thu, Feb 06, 2025 at 04:52:24PM +0530, Mohammad Rafi Shaik wrote:
> Added qcom_swrm_set_channel_map api to set the master channel mask for
> TX and RX paths based on the provided slots.
>
> Added a new field ch_mask to the qcom_swrm_port_config structure.
> This field is used to store the master channel mask, which allows more
> flexible to configure channel mask in runtime for specific active
> soundwire ports.
>
> Modified the qcom_swrm_port_enable function to configure master
> channel mask. If the ch_mask is set to SWR_INVALID_PARAM or is zero,
> the function will use the default channel mask.
>
> Signed-off-by: Mohammad Rafi Shaik <quic_mohs@...cinc.com>
> Acked-by: Vinod Koul <vkoul@...nel.org>
> ---
> drivers/soundwire/qcom.c | 26 ++++++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
>
> diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
> index 0f45e3404756..295a46dc2be7 100644
> --- a/drivers/soundwire/qcom.c
> +++ b/drivers/soundwire/qcom.c
> @@ -156,6 +156,7 @@ struct qcom_swrm_port_config {
> u8 word_length;
> u8 blk_group_count;
> u8 lane_control;
> + u8 ch_mask;
> };
>
> /*
> @@ -1048,9 +1049,13 @@ static int qcom_swrm_port_enable(struct sdw_bus *bus,
> {
> u32 reg = SWRM_DP_PORT_CTRL_BANK(enable_ch->port_num, bank);
> struct qcom_swrm_ctrl *ctrl = to_qcom_sdw(bus);
> + struct qcom_swrm_port_config *pcfg;
> u32 val;
>
> + pcfg = &ctrl->pconfig[enable_ch->port_num];
> ctrl->reg_read(ctrl, reg, &val);
> + if (pcfg->ch_mask != SWR_INVALID_PARAM && pcfg->ch_mask != 0)
> + enable_ch->ch_mask = pcfg->ch_mask;
>
> if (enable_ch->enable)
> val |= (enable_ch->ch_mask << SWRM_DP_PORT_CTRL_EN_CHAN_SHFT);
> @@ -1270,6 +1275,26 @@ 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);
> + int i;
> +
> + if (tx_slot) {
> + for (i = 0; i < tx_num; i++)
> + ctrl->pconfig[i].ch_mask = tx_slot[i];
> + }
> +
> + if (rx_slot) {
> + for (i = 0; i < rx_num; i++)
> + ctrl->pconfig[i].ch_mask = rx_slot[i];
> + }
> +
> + return 0;
> +}
> +
Could we fix or revert this patch? It's called like this from
sdm845_dai_init():
sound/soc/qcom/sdm845.c
247 /*
248 * Codec SLIMBUS configuration
249 * RX1, RX2, RX3, RX4, RX5, RX6, RX7, RX8, RX9, RX10, RX11, RX12, RX13
250 * TX1, TX2, TX3, TX4, TX5, TX6, TX7, TX8, TX9, TX10, TX11, TX12, TX13
251 * TX14, TX15, TX16
252 */
253 unsigned int rx_ch[SLIM_MAX_RX_PORTS] = {144, 145, 146, 147, 148, 149,
254 150, 151, 152, 153, 154, 155, 156};
255 unsigned int tx_ch[SLIM_MAX_TX_PORTS] = {128, 129, 130, 131, 132, 133,
256 134, 135, 136, 137, 138, 139,
257 140, 141, 142, 143};
[ snip ]
304 for_each_rtd_codec_dais(rtd, i, codec_dai) {
305 rval = snd_soc_dai_set_channel_map(codec_dai,
306 ARRAY_SIZE(tx_ch),
307 tx_ch,
308 ARRAY_SIZE(rx_ch),
309 rx_ch);
310 if (rval != 0 && rval != -ENOTSUPP)
311 return rval;
There are 3 bugs:
Bug #1:
The zeroeth element of ctrl->pconfig[] is supposed to be unused. We
start counting at 1. However this code sets ctrl->pconfig[0].ch_mask = 128.
Bug #2:
There are SLIM_MAX_TX_PORTS (16) elements in tx_ch[] array but only
QCOM_SDW_MAX_PORTS + 1 (15) in the ctrl->pconfig[] array so it corrupts
memory like Yongqin Liu pointed out.
Bug 3:
Like Jie Gan pointed out, it erases all the tx information with the rx
information.
regards,
dan carpenter
Powered by blists - more mailing lists