Re: [PATCH v6 15/15] ASoC: msm8916-wcd-analog: add quirk for cajon 2.0

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The 20ms delay and TEST bit clear in `POST_PMD` for HPHL/HPHR PAs execute before the digital codec has powered down, contradicting the hardware requirement and potentially violating hardware constraints.
--

--- Patch [15]: [PATCH v6 15/15] ASoC: msm8916-wcd-analog: add quirk for cajon 2.0 ---
commit 3ee3737216b7e26df5f18202ad771c230e2fa0e3
Author: Richard Acayan <mailingradian@gmail.com>

ASoC: msm8916-wcd-analog: add quirk for cajon 2.0

The codec version CAJON_2_0 on the Snapdragon 670 requires touching the
HPH test registers. Add the quirk so this driver can also support
SDM670.

> diff --git a/sound/soc/codecs/msm8916-wcd-analog.c b/sound/soc/codecs/msm8916-wcd-analog.c
> index b9325290c28d9..87f8a47cc293f 100644
> --- a/sound/soc/codecs/msm8916-wcd-analog.c
> +++ b/sound/soc/codecs/msm8916-wcd-analog.c
[ ... ]
> @@ -705,6 +707,59 @@ static int pm8916_wcd_analog_enable_ear_pa(struct snd_soc_dapm_widget *w,
>  	return 0;
>  }
>  
> +static int pm8916_wcd_analog_enable_hphl_pa(struct snd_soc_dapm_widget *w,
> +					    struct snd_kcontrol *kcontrol,
> +					    int event)
> +{
[ ... ]
> +	switch (event) {
> +	case SND_SOC_DAPM_POST_PMU:
> +		usleep_range(7000, 7100);
> +		snd_soc_component_update_bits(component, CDC_A_RX_HPH_L_TEST,
> +				0x04, 0x04);
> +		break;
> +	case SND_SOC_DAPM_POST_PMD:
> +		/* wait 20 ms after the digital codec has powered down */
> +		msleep(20);
> +		snd_soc_component_update_bits(component, CDC_A_RX_HPH_L_TEST,
> +				0x04, 0x00);
> +		break;

[Severity: High]
Will this POST_PMD event actually wait until after the digital codec has
powered down?

During the DAPM power-down sequence, PGAs are processed (sequence 6) before
Mixers (sequence 7), DACs (sequence 8), and AIFs (sequence 12). Since this
event is attached to the "HPHL PA" PGA widget, the POST_PMD event will execute
immediately after the PA powers down, which is before the digital codec (DACs,
Mixers, AIFs) powers down.

Could this premature delay and test bit clearing cause audible pops or
hardware instability?

> +	}
> +	return 0;
> +}
> +
> +static int pm8916_wcd_analog_enable_hphr_pa(struct snd_soc_dapm_widget *w,
> +					    struct snd_kcontrol *kcontrol,
> +					    int event)
> +{
[ ... ]
> +	switch (event) {
> +	case SND_SOC_DAPM_POST_PMU:
> +		usleep_range(7000, 7100);
> +		snd_soc_component_update_bits(component, CDC_A_RX_HPH_R_TEST,
> +				0x04, 0x04);
> +		break;
> +	case SND_SOC_DAPM_POST_PMD:
> +		msleep(20);
> +		snd_soc_component_update_bits(component, CDC_A_RX_HPH_R_TEST,
> +				0x04, 0x00);
> +		break;

[Severity: High]
Similarly here, does this execute before the digital codec powers down?

Since "HPHR PA" is also a PGA widget, it seems the 20ms sleep and TEST bit
clear will occur while the digital codec is still powered on, just like the
left channel.

> +	}
> +	return 0;
> +}
[ ... ]
> @@ -954,11 +1009,15 @@ static const struct snd_soc_dapm_widget pm8916_wcd_analog_dapm_widgets[] = {
>  	SND_SOC_DAPM_MUX("EAR_S", SND_SOC_NOPM, 0, 0, &ear_mux),
>  	SND_SOC_DAPM_SUPPLY("EAR CP", CDC_A_NCP_EN, 4, 0, NULL, 0),
>  
> -	SND_SOC_DAPM_PGA("HPHL PA", CDC_A_RX_HPH_CNP_EN, 5, 0, NULL, 0),
> +	SND_SOC_DAPM_PGA_E("HPHL PA", CDC_A_RX_HPH_CNP_EN, 5, 0, NULL, 0,
> +			   pm8916_wcd_analog_enable_hphl_pa,
> +			   SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_POST_PMD),
>  	SND_SOC_DAPM_MUX("HPHL", SND_SOC_NOPM, 0, 0, &hphl_mux),
>  	SND_SOC_DAPM_MIXER("HPHL DAC", CDC_A_RX_HPH_L_PA_DAC_CTL, 3, 0, NULL,
>  			   0),
> -	SND_SOC_DAPM_PGA("HPHR PA", CDC_A_RX_HPH_CNP_EN, 4, 0, NULL, 0),
> +	SND_SOC_DAPM_PGA_E("HPHR PA", CDC_A_RX_HPH_CNP_EN, 4, 0, NULL, 0,
> +			   pm8916_wcd_analog_enable_hphr_pa,
> +			   SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_POST_PMD),

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260730174353.108023-1-mailingradian@gmail.com?part=15