[v3] ASoC: codecs: lpass-wsa: fix VI capture setup.

[PATCH v3 2/2] ASoC: codecs:lpass-wsa-macro: Fix logic of enabling vi channels

Posted by srinivas.kandagatla@linaro.org 1 week ago

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

Existing code only configures one of WSA_MACRO_TX0 or WSA_MACRO_TX1
paths eventhough we enable both of them. Fix this bug by adding proper
checks and rearranging some of the common code to able to allow setting
both TX0 and TX1 paths

Without this patch only one channel gets enabled in VI path instead of 2
channels. End result would be 1 channel recording instead of 2.

Fixes: 2c4066e5d428 ("ASoC: codecs: lpass-wsa-macro: add dapm widgets and route")
Cc: stable@vger.kernel.org
Co-developed-by: Manikantan R <quic_manrav@quicinc.com>
Signed-off-by: Manikantan R <quic_manrav@quicinc.com>
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 sound/soc/codecs/lpass-wsa-macro.c | 112 +++++++++++++++++------------
 1 file changed, 68 insertions(+), 44 deletions(-)

diff --git a/sound/soc/codecs/lpass-wsa-macro.c b/sound/soc/codecs/lpass-wsa-macro.c
index ac119847bc22..c9e7f185f2bc 100644
--- a/sound/soc/codecs/lpass-wsa-macro.c
+++ b/sound/soc/codecs/lpass-wsa-macro.c
@@ -1469,46 +1469,11 @@ static int wsa_macro_mclk_event(struct snd_soc_dapm_widget *w,
 	return 0;
 }
 
-static int wsa_macro_enable_vi_feedback(struct snd_soc_dapm_widget *w,
-					struct snd_kcontrol *kcontrol,
-					int event)
-{
-	struct snd_soc_component *component = snd_soc_dapm_to_component(w->dapm);
-	struct wsa_macro *wsa = snd_soc_component_get_drvdata(component);
-	u32 tx_reg0, tx_reg1;
-	u32 rate_val;
 
-	switch (wsa->pcm_rate_vi) {
-	case 8000:
-		rate_val = CDC_WSA_TX_SPKR_PROT_PCM_RATE_8K;
-		break;
-	case 16000:
-		rate_val = CDC_WSA_TX_SPKR_PROT_PCM_RATE_16K;
-		break;
-	case 24000:
-		rate_val = CDC_WSA_TX_SPKR_PROT_PCM_RATE_24K;
-		break;
-	case 32000:
-		rate_val = CDC_WSA_TX_SPKR_PROT_PCM_RATE_32K;
-		break;
-	case 48000:
-		rate_val = CDC_WSA_TX_SPKR_PROT_PCM_RATE_48K;
-		break;
-	default:
-		rate_val = CDC_WSA_TX_SPKR_PROT_PCM_RATE_8K;
-		break;
-	}
-
-	if (test_bit(WSA_MACRO_TX0, &wsa->active_ch_mask[WSA_MACRO_AIF_VI])) {
-		tx_reg0 = CDC_WSA_TX0_SPKR_PROT_PATH_CTL;
-		tx_reg1 = CDC_WSA_TX1_SPKR_PROT_PATH_CTL;
-	} else if (test_bit(WSA_MACRO_TX1, &wsa->active_ch_mask[WSA_MACRO_AIF_VI])) {
-		tx_reg0 = CDC_WSA_TX2_SPKR_PROT_PATH_CTL;
-		tx_reg1 = CDC_WSA_TX3_SPKR_PROT_PATH_CTL;
-	}
-
-	switch (event) {
-	case SND_SOC_DAPM_POST_PMU:
+static void wsa_macro_enable_disable_vi_sense(struct snd_soc_component *component, bool enable,
+						u32 tx_reg0, u32 tx_reg1, u32 val)
+{
+	if (enable) {
 		/* Enable V&I sensing */
 		snd_soc_component_update_bits(component, tx_reg0,
 					      CDC_WSA_TX_SPKR_PROT_RESET_MASK,
@@ -1518,10 +1483,10 @@ static int wsa_macro_enable_vi_feedback(struct snd_soc_dapm_widget *w,
 					      CDC_WSA_TX_SPKR_PROT_RESET);
 		snd_soc_component_update_bits(component, tx_reg0,
 					      CDC_WSA_TX_SPKR_PROT_PCM_RATE_MASK,
-					      rate_val);
+					      val);
 		snd_soc_component_update_bits(component, tx_reg1,
 					      CDC_WSA_TX_SPKR_PROT_PCM_RATE_MASK,
-					      rate_val);
+					      val);
 		snd_soc_component_update_bits(component, tx_reg0,
 					      CDC_WSA_TX_SPKR_PROT_CLK_EN_MASK,
 					      CDC_WSA_TX_SPKR_PROT_CLK_ENABLE);
@@ -1534,9 +1499,7 @@ static int wsa_macro_enable_vi_feedback(struct snd_soc_dapm_widget *w,
 		snd_soc_component_update_bits(component, tx_reg1,
 					      CDC_WSA_TX_SPKR_PROT_RESET_MASK,
 					      CDC_WSA_TX_SPKR_PROT_NO_RESET);
-		break;
-	case SND_SOC_DAPM_POST_PMD:
-		/* Disable V&I sensing */
+	} else {
 		snd_soc_component_update_bits(component, tx_reg0,
 					      CDC_WSA_TX_SPKR_PROT_RESET_MASK,
 					      CDC_WSA_TX_SPKR_PROT_RESET);
@@ -1549,6 +1512,67 @@ static int wsa_macro_enable_vi_feedback(struct snd_soc_dapm_widget *w,
 		snd_soc_component_update_bits(component, tx_reg1,
 					      CDC_WSA_TX_SPKR_PROT_CLK_EN_MASK,
 					      CDC_WSA_TX_SPKR_PROT_CLK_DISABLE);
+	}
+}
+
+static void wsa_macro_enable_disable_vi_feedback(struct snd_soc_component *component,
+						 bool enable, u32 rate)
+{
+	struct wsa_macro *wsa = snd_soc_component_get_drvdata(component);
+	u32 tx_reg0, tx_reg1;
+
+	if (test_bit(WSA_MACRO_TX0, &wsa->active_ch_mask[WSA_MACRO_AIF_VI])) {
+		tx_reg0 = CDC_WSA_TX0_SPKR_PROT_PATH_CTL;
+		tx_reg1 = CDC_WSA_TX1_SPKR_PROT_PATH_CTL;
+		wsa_macro_enable_disable_vi_sense(component, enable, tx_reg0, tx_reg1, rate);
+	}
+
+	if (test_bit(WSA_MACRO_TX1, &wsa->active_ch_mask[WSA_MACRO_AIF_VI])) {
+		tx_reg0 = CDC_WSA_TX2_SPKR_PROT_PATH_CTL;
+		tx_reg1 = CDC_WSA_TX3_SPKR_PROT_PATH_CTL;
+		wsa_macro_enable_disable_vi_sense(component, enable, tx_reg0, tx_reg1, rate);
+
+	}
+
+}
+
+static int wsa_macro_enable_vi_feedback(struct snd_soc_dapm_widget *w,
+					struct snd_kcontrol *kcontrol,
+					int event)
+{
+	struct snd_soc_component *component = snd_soc_dapm_to_component(w->dapm);
+	struct wsa_macro *wsa = snd_soc_component_get_drvdata(component);
+	u32 rate_val;
+
+	switch (wsa->pcm_rate_vi) {
+	case 8000:
+		rate_val = CDC_WSA_TX_SPKR_PROT_PCM_RATE_8K;
+		break;
+	case 16000:
+		rate_val = CDC_WSA_TX_SPKR_PROT_PCM_RATE_16K;
+		break;
+	case 24000:
+		rate_val = CDC_WSA_TX_SPKR_PROT_PCM_RATE_24K;
+		break;
+	case 32000:
+		rate_val = CDC_WSA_TX_SPKR_PROT_PCM_RATE_32K;
+		break;
+	case 48000:
+		rate_val = CDC_WSA_TX_SPKR_PROT_PCM_RATE_48K;
+		break;
+	default:
+		rate_val = CDC_WSA_TX_SPKR_PROT_PCM_RATE_8K;
+		break;
+	}
+
+	switch (event) {
+	case SND_SOC_DAPM_POST_PMU:
+		/* Enable V&I sensing */
+		wsa_macro_enable_disable_vi_feedback(component, true, rate_val);
+		break;
+	case SND_SOC_DAPM_POST_PMD:
+		/* Disable V&I sensing */
+		wsa_macro_enable_disable_vi_feedback(component, false, rate_val);
 		break;
 	}
 
-- 
2.39.5

Re: [PATCH v3 2/2] ASoC: codecs:lpass-wsa-macro: Fix logic of enabling vi channels

Posted by Dmitry Baryshkov 1 week ago

On Thu, Apr 03, 2025 at 01:42:47PM +0100, srinivas.kandagatla@linaro.org wrote:
> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> 
> Existing code only configures one of WSA_MACRO_TX0 or WSA_MACRO_TX1
> paths eventhough we enable both of them. Fix this bug by adding proper
> checks and rearranging some of the common code to able to allow setting
> both TX0 and TX1 paths
> 
> Without this patch only one channel gets enabled in VI path instead of 2
> channels. End result would be 1 channel recording instead of 2.

Could you please rearrange the code to make the patch more obvious?

> 
> Fixes: 2c4066e5d428 ("ASoC: codecs: lpass-wsa-macro: add dapm widgets and route")
> Cc: stable@vger.kernel.org
> Co-developed-by: Manikantan R <quic_manrav@quicinc.com>
> Signed-off-by: Manikantan R <quic_manrav@quicinc.com>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
>  sound/soc/codecs/lpass-wsa-macro.c | 112 +++++++++++++++++------------
>  1 file changed, 68 insertions(+), 44 deletions(-)
> 
> diff --git a/sound/soc/codecs/lpass-wsa-macro.c b/sound/soc/codecs/lpass-wsa-macro.c
> index ac119847bc22..c9e7f185f2bc 100644
> --- a/sound/soc/codecs/lpass-wsa-macro.c
> +++ b/sound/soc/codecs/lpass-wsa-macro.c
> @@ -1469,46 +1469,11 @@ static int wsa_macro_mclk_event(struct snd_soc_dapm_widget *w,
>  	return 0;
>  }
>  
> -static int wsa_macro_enable_vi_feedback(struct snd_soc_dapm_widget *w,
> -					struct snd_kcontrol *kcontrol,
> -					int event)
> -{
> -	struct snd_soc_component *component = snd_soc_dapm_to_component(w->dapm);
> -	struct wsa_macro *wsa = snd_soc_component_get_drvdata(component);
> -	u32 tx_reg0, tx_reg1;
> -	u32 rate_val;
>  
> -	switch (wsa->pcm_rate_vi) {
> -	case 8000:
> -		rate_val = CDC_WSA_TX_SPKR_PROT_PCM_RATE_8K;
> -		break;
> -	case 16000:
> -		rate_val = CDC_WSA_TX_SPKR_PROT_PCM_RATE_16K;
> -		break;
> -	case 24000:
> -		rate_val = CDC_WSA_TX_SPKR_PROT_PCM_RATE_24K;
> -		break;
> -	case 32000:
> -		rate_val = CDC_WSA_TX_SPKR_PROT_PCM_RATE_32K;
> -		break;
> -	case 48000:
> -		rate_val = CDC_WSA_TX_SPKR_PROT_PCM_RATE_48K;
> -		break;
> -	default:
> -		rate_val = CDC_WSA_TX_SPKR_PROT_PCM_RATE_8K;
> -		break;
> -	}

This can go to the wsa_macro_enable_disable_vi_sense().

> -
> -	if (test_bit(WSA_MACRO_TX0, &wsa->active_ch_mask[WSA_MACRO_AIF_VI])) {
> -		tx_reg0 = CDC_WSA_TX0_SPKR_PROT_PATH_CTL;
> -		tx_reg1 = CDC_WSA_TX1_SPKR_PROT_PATH_CTL;
> -	} else if (test_bit(WSA_MACRO_TX1, &wsa->active_ch_mask[WSA_MACRO_AIF_VI])) {
> -		tx_reg0 = CDC_WSA_TX2_SPKR_PROT_PATH_CTL;
> -		tx_reg1 = CDC_WSA_TX3_SPKR_PROT_PATH_CTL;
> -	}
> -
> -	switch (event) {
> -	case SND_SOC_DAPM_POST_PMU:
> +static void wsa_macro_enable_disable_vi_sense(struct snd_soc_component *component, bool enable,
> +						u32 tx_reg0, u32 tx_reg1, u32 val)
> +{
> +	if (enable) {
>  		/* Enable V&I sensing */
>  		snd_soc_component_update_bits(component, tx_reg0,
>  					      CDC_WSA_TX_SPKR_PROT_RESET_MASK,
> @@ -1518,10 +1483,10 @@ static int wsa_macro_enable_vi_feedback(struct snd_soc_dapm_widget *w,
>  					      CDC_WSA_TX_SPKR_PROT_RESET);
>  		snd_soc_component_update_bits(component, tx_reg0,
>  					      CDC_WSA_TX_SPKR_PROT_PCM_RATE_MASK,
> -					      rate_val);
> +					      val);

No need for extra renames, they complicate reviewing.

>  		snd_soc_component_update_bits(component, tx_reg1,
>  					      CDC_WSA_TX_SPKR_PROT_PCM_RATE_MASK,
> -					      rate_val);
> +					      val);
>  		snd_soc_component_update_bits(component, tx_reg0,
>  					      CDC_WSA_TX_SPKR_PROT_CLK_EN_MASK,
>  					      CDC_WSA_TX_SPKR_PROT_CLK_ENABLE);
> @@ -1534,9 +1499,7 @@ static int wsa_macro_enable_vi_feedback(struct snd_soc_dapm_widget *w,
>  		snd_soc_component_update_bits(component, tx_reg1,
>  					      CDC_WSA_TX_SPKR_PROT_RESET_MASK,
>  					      CDC_WSA_TX_SPKR_PROT_NO_RESET);
> -		break;
> -	case SND_SOC_DAPM_POST_PMD:
> -		/* Disable V&I sensing */
> +	} else {
>  		snd_soc_component_update_bits(component, tx_reg0,
>  					      CDC_WSA_TX_SPKR_PROT_RESET_MASK,
>  					      CDC_WSA_TX_SPKR_PROT_RESET);
> @@ -1549,6 +1512,67 @@ static int wsa_macro_enable_vi_feedback(struct snd_soc_dapm_widget *w,
>  		snd_soc_component_update_bits(component, tx_reg1,
>  					      CDC_WSA_TX_SPKR_PROT_CLK_EN_MASK,
>  					      CDC_WSA_TX_SPKR_PROT_CLK_DISABLE);
> +	}
> +}
> +
> +static void wsa_macro_enable_disable_vi_feedback(struct snd_soc_component *component,
> +						 bool enable, u32 rate)
> +{
> +	struct wsa_macro *wsa = snd_soc_component_get_drvdata(component);
> +	u32 tx_reg0, tx_reg1;
> +
> +	if (test_bit(WSA_MACRO_TX0, &wsa->active_ch_mask[WSA_MACRO_AIF_VI])) {
> +		tx_reg0 = CDC_WSA_TX0_SPKR_PROT_PATH_CTL;
> +		tx_reg1 = CDC_WSA_TX1_SPKR_PROT_PATH_CTL;
> +		wsa_macro_enable_disable_vi_sense(component, enable, tx_reg0, tx_reg1, rate);

As you are refactoring this piece of code, do you need tx_reg0 / tx_reg1
variables? Can you inline them instead?

> +	}
> +
> +	if (test_bit(WSA_MACRO_TX1, &wsa->active_ch_mask[WSA_MACRO_AIF_VI])) {
> +		tx_reg0 = CDC_WSA_TX2_SPKR_PROT_PATH_CTL;
> +		tx_reg1 = CDC_WSA_TX3_SPKR_PROT_PATH_CTL;
> +		wsa_macro_enable_disable_vi_sense(component, enable, tx_reg0, tx_reg1, rate);
> +
> +	}
> +

Extra empty line.

> +}
> +
> +static int wsa_macro_enable_vi_feedback(struct snd_soc_dapm_widget *w,
> +					struct snd_kcontrol *kcontrol,
> +					int event)
> +{
> +	struct snd_soc_component *component = snd_soc_dapm_to_component(w->dapm);
> +	struct wsa_macro *wsa = snd_soc_component_get_drvdata(component);
> +	u32 rate_val;
> +
> +	switch (wsa->pcm_rate_vi) {
> +	case 8000:
> +		rate_val = CDC_WSA_TX_SPKR_PROT_PCM_RATE_8K;
> +		break;
> +	case 16000:
> +		rate_val = CDC_WSA_TX_SPKR_PROT_PCM_RATE_16K;
> +		break;
> +	case 24000:
> +		rate_val = CDC_WSA_TX_SPKR_PROT_PCM_RATE_24K;
> +		break;
> +	case 32000:
> +		rate_val = CDC_WSA_TX_SPKR_PROT_PCM_RATE_32K;
> +		break;
> +	case 48000:
> +		rate_val = CDC_WSA_TX_SPKR_PROT_PCM_RATE_48K;
> +		break;
> +	default:
> +		rate_val = CDC_WSA_TX_SPKR_PROT_PCM_RATE_8K;
> +		break;
> +	}
> +
> +	switch (event) {
> +	case SND_SOC_DAPM_POST_PMU:
> +		/* Enable V&I sensing */
> +		wsa_macro_enable_disable_vi_feedback(component, true, rate_val);
> +		break;
> +	case SND_SOC_DAPM_POST_PMD:
> +		/* Disable V&I sensing */
> +		wsa_macro_enable_disable_vi_feedback(component, false, rate_val);
>  		break;
>  	}
>  
> -- 
> 2.39.5
> 

-- 
With best wishes
Dmitry

Re: [PATCH v3 2/2] ASoC: codecs:lpass-wsa-macro: Fix logic of enabling vi channels

Posted by Srinivas Kandagatla 1 week ago


On 03/04/2025 15:35, Dmitry Baryshkov wrote:
> On Thu, Apr 03, 2025 at 01:42:47PM +0100, srinivas.kandagatla@linaro.org wrote:
>> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>>
>> Existing code only configures one of WSA_MACRO_TX0 or WSA_MACRO_TX1
>> paths eventhough we enable both of them. Fix this bug by adding proper
>> checks and rearranging some of the common code to able to allow setting
>> both TX0 and TX1 paths
>>
>> Without this patch only one channel gets enabled in VI path instead of 2
>> channels. End result would be 1 channel recording instead of 2.
> 
> Could you please rearrange the code to make the patch more obvious?
Will try it in next version.

> 
>>
>> Fixes: 2c4066e5d428 ("ASoC: codecs: lpass-wsa-macro: add dapm widgets and route")
>> Cc: stable@vger.kernel.org
>> Co-developed-by: Manikantan R <quic_manrav@quicinc.com>
>> Signed-off-by: Manikantan R <quic_manrav@quicinc.com>
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>> ---
>>   sound/soc/codecs/lpass-wsa-macro.c | 112 +++++++++++++++++------------
>>   1 file changed, 68 insertions(+), 44 deletions(-)
>>
>> diff --git a/sound/soc/codecs/lpass-wsa-macro.c b/sound/soc/codecs/lpass-wsa-macro.c
>> index ac119847bc22..c9e7f185f2bc 100644
>> --- a/sound/soc/codecs/lpass-wsa-macro.c
>> +++ b/sound/soc/codecs/lpass-wsa-macro.c
>> @@ -1469,46 +1469,11 @@ static int wsa_macro_mclk_event(struct snd_soc_dapm_widget *w,
>>   	return 0;
>>   }
>>   
>> -static int wsa_macro_enable_vi_feedback(struct snd_soc_dapm_widget *w,
>> -					struct snd_kcontrol *kcontrol,
>> -					int event)
>> -{
>> -	struct snd_soc_component *component = snd_soc_dapm_to_component(w->dapm);
>> -	struct wsa_macro *wsa = snd_soc_component_get_drvdata(component);
>> -	u32 tx_reg0, tx_reg1;
>> -	u32 rate_val;
>>   
>> -	switch (wsa->pcm_rate_vi) {
>> -	case 8000:
>> -		rate_val = CDC_WSA_TX_SPKR_PROT_PCM_RATE_8K;
>> -		break;
>> -	case 16000:
>> -		rate_val = CDC_WSA_TX_SPKR_PROT_PCM_RATE_16K;
>> -		break;
>> -	case 24000:
>> -		rate_val = CDC_WSA_TX_SPKR_PROT_PCM_RATE_24K;
>> -		break;
>> -	case 32000:
>> -		rate_val = CDC_WSA_TX_SPKR_PROT_PCM_RATE_32K;
>> -		break;
>> -	case 48000:
>> -		rate_val = CDC_WSA_TX_SPKR_PROT_PCM_RATE_48K;
>> -		break;
>> -	default:
>> -		rate_val = CDC_WSA_TX_SPKR_PROT_PCM_RATE_8K;
>> -		break;
>> -	}
> 
> This can go to the wsa_macro_enable_disable_vi_sense().
Not sure I undestood this, v4 looks much clearner without this hunk.

> 
>> -
>> -	if (test_bit(WSA_MACRO_TX0, &wsa->active_ch_mask[WSA_MACRO_AIF_VI])) {
>> -		tx_reg0 = CDC_WSA_TX0_SPKR_PROT_PATH_CTL;
>> -		tx_reg1 = CDC_WSA_TX1_SPKR_PROT_PATH_CTL;
>> -	} else if (test_bit(WSA_MACRO_TX1, &wsa->active_ch_mask[WSA_MACRO_AIF_VI])) {
>> -		tx_reg0 = CDC_WSA_TX2_SPKR_PROT_PATH_CTL;
>> -		tx_reg1 = CDC_WSA_TX3_SPKR_PROT_PATH_CTL;
>> -	}
>> -
>> -	switch (event) {
>> -	case SND_SOC_DAPM_POST_PMU:
>> +static void wsa_macro_enable_disable_vi_sense(struct snd_soc_component *component, bool enable,
>> +						u32 tx_reg0, u32 tx_reg1, u32 val)
>> +{
>> +	if (enable) {
>>   		/* Enable V&I sensing */
>>   		snd_soc_component_update_bits(component, tx_reg0,
>>   					      CDC_WSA_TX_SPKR_PROT_RESET_MASK,
>> @@ -1518,10 +1483,10 @@ static int wsa_macro_enable_vi_feedback(struct snd_soc_dapm_widget *w,
>>   					      CDC_WSA_TX_SPKR_PROT_RESET);
>>   		snd_soc_component_update_bits(component, tx_reg0,
>>   					      CDC_WSA_TX_SPKR_PROT_PCM_RATE_MASK,
>> -					      rate_val);
>> +					      val);
> 
> No need for extra renames, they complicate reviewing.
> 
>>   		snd_soc_component_update_bits(component, tx_reg1,
>>   					      CDC_WSA_TX_SPKR_PROT_PCM_RATE_MASK,
>> -					      rate_val);
>> +					      val);
>>   		snd_soc_component_update_bits(component, tx_reg0,
>>   					      CDC_WSA_TX_SPKR_PROT_CLK_EN_MASK,
>>   					      CDC_WSA_TX_SPKR_PROT_CLK_ENABLE);
>> @@ -1534,9 +1499,7 @@ static int wsa_macro_enable_vi_feedback(struct snd_soc_dapm_widget *w,
>>   		snd_soc_component_update_bits(component, tx_reg1,
>>   					      CDC_WSA_TX_SPKR_PROT_RESET_MASK,
>>   					      CDC_WSA_TX_SPKR_PROT_NO_RESET);
>> -		break;
>> -	case SND_SOC_DAPM_POST_PMD:
>> -		/* Disable V&I sensing */
>> +	} else {
>>   		snd_soc_component_update_bits(component, tx_reg0,
>>   					      CDC_WSA_TX_SPKR_PROT_RESET_MASK,
>>   					      CDC_WSA_TX_SPKR_PROT_RESET);
>> @@ -1549,6 +1512,67 @@ static int wsa_macro_enable_vi_feedback(struct snd_soc_dapm_widget *w,
>>   		snd_soc_component_update_bits(component, tx_reg1,
>>   					      CDC_WSA_TX_SPKR_PROT_CLK_EN_MASK,
>>   					      CDC_WSA_TX_SPKR_PROT_CLK_DISABLE);
>> +	}
>> +}
>> +
>> +static void wsa_macro_enable_disable_vi_feedback(struct snd_soc_component *component,
>> +						 bool enable, u32 rate)
>> +{
>> +	struct wsa_macro *wsa = snd_soc_component_get_drvdata(component);
>> +	u32 tx_reg0, tx_reg1;
>> +
>> +	if (test_bit(WSA_MACRO_TX0, &wsa->active_ch_mask[WSA_MACRO_AIF_VI])) {
>> +		tx_reg0 = CDC_WSA_TX0_SPKR_PROT_PATH_CTL;
>> +		tx_reg1 = CDC_WSA_TX1_SPKR_PROT_PATH_CTL;
>> +		wsa_macro_enable_disable_vi_sense(component, enable, tx_reg0, tx_reg1, rate);
> 
> As you are refactoring this piece of code, do you need tx_reg0 / tx_reg1
> variables? Can you inline them instead?

Ack.

> 
>> +	}
>> +
>> +	if (test_bit(WSA_MACRO_TX1, &wsa->active_ch_mask[WSA_MACRO_AIF_VI])) {
>> +		tx_reg0 = CDC_WSA_TX2_SPKR_PROT_PATH_CTL;
>> +		tx_reg1 = CDC_WSA_TX3_SPKR_PROT_PATH_CTL;
>> +		wsa_macro_enable_disable_vi_sense(component, enable, tx_reg0, tx_reg1, rate);
>> +
>> +	}
>> +
> 
> Extra empty line.

Ack

> 
>> +}
>> +
>> +static int wsa_macro_enable_vi_feedback(struct snd_soc_dapm_widget *w,
>> +					struct snd_kcontrol *kcontrol,
>> +					int event)
>> +{
>> +	struct snd_soc_component *component = snd_soc_dapm_to_component(w->dapm);
>> +	struct wsa_macro *wsa = snd_soc_component_get_drvdata(component);
>> +	u32 rate_val;
>> +
>> +	switch (wsa->pcm_rate_vi) {
>> +	case 8000:
>> +		rate_val = CDC_WSA_TX_SPKR_PROT_PCM_RATE_8K;
>> +		break;
>> +	case 16000:
>> +		rate_val = CDC_WSA_TX_SPKR_PROT_PCM_RATE_16K;
>> +		break;
>> +	case 24000:
>> +		rate_val = CDC_WSA_TX_SPKR_PROT_PCM_RATE_24K;
>> +		break;
>> +	case 32000:
>> +		rate_val = CDC_WSA_TX_SPKR_PROT_PCM_RATE_32K;
>> +		break;
>> +	case 48000:
>> +		rate_val = CDC_WSA_TX_SPKR_PROT_PCM_RATE_48K;
>> +		break;
>> +	default:
>> +		rate_val = CDC_WSA_TX_SPKR_PROT_PCM_RATE_8K;
>> +		break;
>> +	}
>> +
>> +	switch (event) {
>> +	case SND_SOC_DAPM_POST_PMU:
>> +		/* Enable V&I sensing */
>> +		wsa_macro_enable_disable_vi_feedback(component, true, rate_val);
>> +		break;
>> +	case SND_SOC_DAPM_POST_PMD:
>> +		/* Disable V&I sensing */
>> +		wsa_macro_enable_disable_vi_feedback(component, false, rate_val);
>>   		break;
>>   	}
>>   
>> -- 
>> 2.39.5
>>
>

[PATCH v3 1/2] ASoC: codecs:lpass-wsa-macro: Fix vi feedback rate
[PATCH v3 2/2] ASoC: codecs:lpass-wsa-macro: Fix logic of enabling vi channels