Re: [PATCH v2 3/3] ASoC: cs42l43: Improve pop performance of the input power up

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] ASoC: cs42l43: missing DAPM lock on Decimator 5 and 6 HPF controls
--

commit 8cf1c31ea6b0941fdadc51d4efe5a6f1e81ead1b
Author: Charles Keepax <ckeepax@opensource.cirrus.com>

ASoC: cs42l43: Improve pop performance of the input power up

This commit adjusts the decimator high pass filter settings and adds a 150ms
settling delay during microphone power up to reduce startup pop noises, while
converting the Decimator 1..4 HPF controls to take the DAPM lock.

> diff --git a/sound/soc/codecs/cs42l43.c b/sound/soc/codecs/cs42l43.c
> index 8fccddf7a1b22..43758f790cb0f 100644
> --- a/sound/soc/codecs/cs42l43.c
> +++ b/sound/soc/codecs/cs42l43.c
> @@ -1345,19 +1345,27 @@ static const struct snd_kcontrol_new cs42l43_controls[] = {
>  	SOC_ENUM("Decimator 3 WNF Corner Frequency", cs42l43_dec3_wnf_corner),
>  	SOC_ENUM("Decimator 4 WNF Corner Frequency", cs42l43_dec4_wnf_corner),
>  
> -	SOC_SINGLE("Decimator 1 HPF Switch", CS42L43_DECIM_HPF_WNF_CTRL1,
> -		   CS42L43_DECIM_HPF_EN_SHIFT, 1, 0),
> -	SOC_SINGLE("Decimator 2 HPF Switch", CS42L43_DECIM_HPF_WNF_CTRL2,
> -		   CS42L43_DECIM_HPF_EN_SHIFT, 1, 0),
> -	SOC_SINGLE("Decimator 3 HPF Switch", CS42L43_DECIM_HPF_WNF_CTRL3,
> -		   CS42L43_DECIM_HPF_EN_SHIFT, 1, 0),
> -	SOC_SINGLE("Decimator 4 HPF Switch", CS42L43_DECIM_HPF_WNF_CTRL4,
> -		   CS42L43_DECIM_HPF_EN_SHIFT, 1, 0),
> +	SOC_SINGLE_EXT("Decimator 1 HPF Switch", CS42L43_DECIM_HPF_WNF_CTRL1,
> +		       CS42L43_DECIM_HPF_EN_SHIFT, 1, 0,
> +		       cs42l43_dapm_get_volsw, cs42l43_dapm_put_volsw),
> +	SOC_SINGLE_EXT("Decimator 2 HPF Switch", CS42L43_DECIM_HPF_WNF_CTRL2,
> +		       CS42L43_DECIM_HPF_EN_SHIFT, 1, 0,
> +		       cs42l43_dapm_get_volsw, cs42l43_dapm_put_volsw),
> +	SOC_SINGLE_EXT("Decimator 3 HPF Switch", CS42L43_DECIM_HPF_WNF_CTRL3,
> +		       CS42L43_DECIM_HPF_EN_SHIFT, 1, 0,
> +		       cs42l43_dapm_get_volsw, cs42l43_dapm_put_volsw),
> +	SOC_SINGLE_EXT("Decimator 4 HPF Switch", CS42L43_DECIM_HPF_WNF_CTRL4,
> +		       CS42L43_DECIM_HPF_EN_SHIFT, 1, 0,
> +		       cs42l43_dapm_get_volsw, cs42l43_dapm_put_volsw),
>  
> -	SOC_ENUM("Decimator 1 HPF Corner Frequency", cs42l43_dec1_hpf_corner),
> -	SOC_ENUM("Decimator 2 HPF Corner Frequency", cs42l43_dec2_hpf_corner),
> -	SOC_ENUM("Decimator 3 HPF Corner Frequency", cs42l43_dec3_hpf_corner),
> -	SOC_ENUM("Decimator 4 HPF Corner Frequency", cs42l43_dec4_hpf_corner),
> +	SOC_ENUM_EXT("Decimator 1 HPF Corner Frequency", cs42l43_dec1_hpf_corner,
> +		     cs42l43_dapm_get_enum, cs42l43_dapm_put_enum),
> +	SOC_ENUM_EXT("Decimator 2 HPF Corner Frequency", cs42l43_dec2_hpf_corner,
> +		     cs42l43_dapm_get_enum, cs42l43_dapm_put_enum),
> +	SOC_ENUM_EXT("Decimator 3 HPF Corner Frequency", cs42l43_dec3_hpf_corner,
> +		     cs42l43_dapm_get_enum, cs42l43_dapm_put_enum),
> +	SOC_ENUM_EXT("Decimator 4 HPF Corner Frequency", cs42l43_dec4_hpf_corner,
> +		     cs42l43_dapm_get_enum, cs42l43_dapm_put_enum),

[ ... ]

> @@ -1770,26 +1785,32 @@ static const struct decim_info cs42l43_decim_info[] = {
>  
>  static const struct decim_info cs42l43b_decim_info[] = {
>  	{
> +		.freg = CS42L43_DECIM_HPF_WNF_CTRL1,
>  		.vreg = CS42L43B_DECIM_VOL_CTRL_CH1_CH2,
>  		.vmask = CS42L43_DECIM1_VD_RAMP_MASK | CS42L43_DECIM1_MUTE_MASK,
>  	},
>  	{
> +		.freg = CS42L43_DECIM_HPF_WNF_CTRL2,
>  		.vreg = CS42L43B_DECIM_VOL_CTRL_CH1_CH2,
>  		.vmask = CS42L43_DECIM2_VD_RAMP_MASK | CS42L43_DECIM2_MUTE_MASK,
>  	},
>  	{
> +		.freg = CS42L43_DECIM_HPF_WNF_CTRL3,
>  		.vreg = CS42L43B_DECIM_VOL_CTRL_CH3_CH4,
>  		.vmask = CS42L43_DECIM3_VD_RAMP_MASK | CS42L43_DECIM3_MUTE_MASK,
>  	},
>  	{
> +		.freg = CS42L43_DECIM_HPF_WNF_CTRL4,
>  		.vreg = CS42L43B_DECIM_VOL_CTRL_CH3_CH4,
>  		.vmask = CS42L43_DECIM4_VD_RAMP_MASK | CS42L43_DECIM4_MUTE_MASK,
>  	},
>  	{
> +		.freg = CS42L43B_DECIM_HPF_WNF_CTRL5,
>  		.vreg = CS42L43B_DECIM_VOL_CTRL_CH5_CH6,
>  		.vmask = CS42L43B_DECIM5_PATH1_VOL_FALL_RATE_MASK | CS42L43B_DECIM5_MUTE_MASK,
>  	},
>  	{
> +		.freg = CS42L43B_DECIM_HPF_WNF_CTRL6,
>  		.vreg = CS42L43B_DECIM_VOL_CTRL_CH5_CH6,
>  		.vmask = CS42L43B_DECIM6_PATH1_VOL_FALL_RATE_MASK | CS42L43B_DECIM6_MUTE_MASK,
>  	}

[Severity: Medium]
As reported by Sashiko on v1 [1], cs42l43b_decim_info[] adds
CS42L43B_DECIM_HPF_WNF_CTRL5 and CS42L43B_DECIM_HPF_WNF_CTRL6 so that
cs42l43_mic_ev() caches, temporarily overrides, and restores them across the
150ms settle delay during PDM2L and PDM2R startup.

While the Decimator 1..4 HPF controls in cs42l43_controls[] were updated to
use the DAPM-locked helpers, the Decimator 5 and 6 HPF controls in
cs42l43_b_controls[] still use plain SOC_SINGLE and SOC_ENUM:

sound/soc/codecs/cs42l43.c:cs42l43_b_controls[] {
	...
	SOC_SINGLE("Decimator 5 HPF Switch", CS42L43B_DECIM_HPF_WNF_CTRL5,
		   CS42L43_DECIM_HPF_EN_SHIFT, 1, 0),
	SOC_SINGLE("Decimator 6 HPF Switch", CS42L43B_DECIM_HPF_WNF_CTRL6,
		   CS42L43_DECIM_HPF_EN_SHIFT, 1, 0),

	SOC_ENUM("Decimator 5 HPF Corner Frequency", cs42l43b_dec5_hpf_corner),
	SOC_ENUM("Decimator 6 HPF Corner Frequency", cs42l43b_dec6_hpf_corner),
};

If userspace reads or writes the Decimator 5 or 6 HPF controls on CS42L43B
while PDM2L or PDM2R is powering up in cs42l43_mic_ev(), can it read back the
temporary startup filter settings or have a concurrent write overwritten when
SND_SOC_DAPM_POST_PMU restores priv->decim_cache[w->shift][1]?

Should the Decimator 5 and 6 HPF Switch and Corner Frequency controls in
cs42l43_b_controls[] also be converted to SOC_SINGLE_EXT and SOC_ENUM_EXT with
cs42l43_dapm_get_volsw, cs42l43_dapm_put_volsw, cs42l43_dapm_get_enum, and
cs42l43_dapm_put_enum?

[1] https://lore.kernel.org/r/sashiko-outbox-193741@kernel.org

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103141857.672537-1-ckeepax@opensource.cirrus.com?part=3