linux-kernel - Re: [PATCH v6 3/4] soundwire: qcom: Add set_channel

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <4ddd6855-3817-4dc4-81c4-d8ddaa039865@oss.qualcomm.com>
Date: Mon, 10 Feb 2025 11:30:01 +0800
From: Jie Gan <jie.gan@....qualcomm.com>
To: Mohammad Rafi Shaik <quic_mohs@...cinc.com>,
        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>
Cc: 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 2/6/2025 7:22 PM, 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];
> +	}
> +
It looks like a hack.
Consider the situation: if(tx_slot) is true and if(rx_slot) is true. So 
the ch_mask always overwritten by rx_slot?

> +	return 0;
I think you dont need the return value here. Just void is ok.

Thanks,
Jie

> +}
> +
>   static int qcom_swrm_startup(struct snd_pcm_substream *substream,
>   			     struct snd_soc_dai *dai)
>   {
> @@ -1306,6 +1331,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 = {