Re: [PATCH 1/2] ALSA: hda/cirrus: Correct the full scale volume set logic

[Takashi Iwai <tiwai@suse.de>]

On Fri, 14 Feb 2025 17:23:25 +0100,
Vitaly Rodionov wrote:
> 
> This patch corrects the full-scale volume setting logic. On certain
> platforms, the full-scale volume bit is required. The current logic
> mistakenly sets this bit and incorrectly clears reserved bit 0, causing
> the headphone output to be muted.
> 
> Signed-off-by: Vitaly Rodionov <vitalyr@opensource.cirrus.com>

Can we have a Fixes tag?


thanks,

Takashi

> ---
>  sound/pci/hda/patch_cs8409-tables.c |  6 +++---
>  sound/pci/hda/patch_cs8409.c        | 20 +++++++++++---------
>  sound/pci/hda/patch_cs8409.h        |  5 +++--
>  3 files changed, 17 insertions(+), 14 deletions(-)
> 
> diff --git a/sound/pci/hda/patch_cs8409-tables.c b/sound/pci/hda/patch_cs8409-tables.c
> index 759f48038273..621f947e3817 100644
> --- a/sound/pci/hda/patch_cs8409-tables.c
> +++ b/sound/pci/hda/patch_cs8409-tables.c
> @@ -121,7 +121,7 @@ static const struct cs8409_i2c_param cs42l42_init_reg_seq[] = {
>  	{ CS42L42_MIXER_CHA_VOL, 0x3F },
>  	{ CS42L42_MIXER_CHB_VOL, 0x3F },
>  	{ CS42L42_MIXER_ADC_VOL, 0x3f },
> -	{ CS42L42_HP_CTL, 0x03 },
> +	{ CS42L42_HP_CTL, 0x0D },
>  	{ CS42L42_MIC_DET_CTL1, 0xB6 },
>  	{ CS42L42_TIPSENSE_CTL, 0xC2 },
>  	{ CS42L42_HS_CLAMP_DISABLE, 0x01 },
> @@ -315,7 +315,7 @@ static const struct cs8409_i2c_param dolphin_c0_init_reg_seq[] = {
>  	{ CS42L42_ASP_TX_SZ_EN, 0x01 },
>  	{ CS42L42_PWR_CTL1, 0x0A },
>  	{ CS42L42_PWR_CTL2, 0x84 },
> -	{ CS42L42_HP_CTL, 0x03 },
> +	{ CS42L42_HP_CTL, 0x0D },
>  	{ CS42L42_MIXER_CHA_VOL, 0x3F },
>  	{ CS42L42_MIXER_CHB_VOL, 0x3F },
>  	{ CS42L42_MIXER_ADC_VOL, 0x3f },
> @@ -371,7 +371,7 @@ static const struct cs8409_i2c_param dolphin_c1_init_reg_seq[] = {
>  	{ CS42L42_ASP_TX_SZ_EN, 0x00 },
>  	{ CS42L42_PWR_CTL1, 0x0E },
>  	{ CS42L42_PWR_CTL2, 0x84 },
> -	{ CS42L42_HP_CTL, 0x01 },
> +	{ CS42L42_HP_CTL, 0x0D },
>  	{ CS42L42_MIXER_CHA_VOL, 0x3F },
>  	{ CS42L42_MIXER_CHB_VOL, 0x3F },
>  	{ CS42L42_MIXER_ADC_VOL, 0x3f },
> diff --git a/sound/pci/hda/patch_cs8409.c b/sound/pci/hda/patch_cs8409.c
> index 614327218634..b760332a4e35 100644
> --- a/sound/pci/hda/patch_cs8409.c
> +++ b/sound/pci/hda/patch_cs8409.c
> @@ -876,7 +876,7 @@ static void cs42l42_resume(struct sub_codec *cs42l42)
>  		{ CS42L42_DET_INT_STATUS2, 0x00 },
>  		{ CS42L42_TSRS_PLUG_STATUS, 0x00 },
>  	};
> -	int fsv_old, fsv_new;
> +	unsigned int fsv;
>  
>  	/* Bring CS42L42 out of Reset */
>  	spec->gpio_data = snd_hda_codec_read(codec, CS8409_PIN_AFG, 0, AC_VERB_GET_GPIO_DATA, 0);
> @@ -893,13 +893,15 @@ static void cs42l42_resume(struct sub_codec *cs42l42)
>  	/* Clear interrupts, by reading interrupt status registers */
>  	cs8409_i2c_bulk_read(cs42l42, irq_regs, ARRAY_SIZE(irq_regs));
>  
> -	fsv_old = cs8409_i2c_read(cs42l42, CS42L42_HP_CTL);
> -	if (cs42l42->full_scale_vol == CS42L42_FULL_SCALE_VOL_0DB)
> -		fsv_new = fsv_old & ~CS42L42_FULL_SCALE_VOL_MASK;
> -	else
> -		fsv_new = fsv_old & CS42L42_FULL_SCALE_VOL_MASK;
> -	if (fsv_new != fsv_old)
> -		cs8409_i2c_write(cs42l42, CS42L42_HP_CTL, fsv_new);
> +	fsv = cs8409_i2c_read(cs42l42, CS42L42_HP_CTL);
> +	if (cs42l42->full_scale_vol) {
> +		// Set the full scale volume bit
> +		fsv |= CS42L42_FULL_SCALE_VOL_MASK;
> +		cs8409_i2c_write(cs42l42, CS42L42_HP_CTL, fsv);
> +	}
> +	// Unmute analog channels A and B
> +	fsv = (fsv & ~CS42L42_ANA_MUTE_AB);
> +	cs8409_i2c_write(cs42l42, CS42L42_HP_CTL, fsv);
>  
>  	/* we have to explicitly allow unsol event handling even during the
>  	 * resume phase so that the jack event is processed properly
> @@ -920,7 +922,7 @@ static void cs42l42_suspend(struct sub_codec *cs42l42)
>  		{ CS42L42_MIXER_CHA_VOL, 0x3F },
>  		{ CS42L42_MIXER_ADC_VOL, 0x3F },
>  		{ CS42L42_MIXER_CHB_VOL, 0x3F },
> -		{ CS42L42_HP_CTL, 0x0F },
> +		{ CS42L42_HP_CTL, 0x0D },
>  		{ CS42L42_ASP_RX_DAI0_EN, 0x00 },
>  		{ CS42L42_ASP_CLK_CFG, 0x00 },
>  		{ CS42L42_PWR_CTL1, 0xFE },
> diff --git a/sound/pci/hda/patch_cs8409.h b/sound/pci/hda/patch_cs8409.h
> index 5e48115caf09..14645d25e70f 100644
> --- a/sound/pci/hda/patch_cs8409.h
> +++ b/sound/pci/hda/patch_cs8409.h
> @@ -230,9 +230,10 @@ enum cs8409_coefficient_index_registers {
>  #define CS42L42_PDN_TIMEOUT_US			(250000)
>  #define CS42L42_PDN_SLEEP_US			(2000)
>  #define CS42L42_INIT_TIMEOUT_MS			(45)
> +#define CS42L42_ANA_MUTE_AB			(0x0C)
>  #define CS42L42_FULL_SCALE_VOL_MASK		(2)
> -#define CS42L42_FULL_SCALE_VOL_0DB		(1)
> -#define CS42L42_FULL_SCALE_VOL_MINUS6DB		(0)
> +#define CS42L42_FULL_SCALE_VOL_0DB		(0)
> +#define CS42L42_FULL_SCALE_VOL_MINUS6DB		(1)
>  
>  /* Dell BULLSEYE / WARLOCK / CYBORG Specific Definitions */
>  
> -- 
> 2.43.0
>