linux-kernel - Re: [PATCH V2] ALSA: hda/conexant: fix Z60MR100 startup pop issue

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <874j3r6db8.wl-tiwai@suse.de>
Date: Thu, 28 Nov 2024 08:13:31 +0100
From: Takashi Iwai <tiwai@...e.de>
To: bo liu <bo.liu@...arytech.com>
Cc: perex@...ex.cz,
	tiwai@...e.com,
	linux-sound@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	feng.liu@...arytech.com
Subject: Re: [PATCH V2] ALSA: hda/conexant: fix Z60MR100 startup pop issue

On Thu, 28 Nov 2024 07:08:12 +0100,
bo liu wrote:
> 
> When Z60MR100 startup, speaker will output a pop. To fix this issue,
> we mute codec by init verbs in bios when system startup, and set GPIO
> to low to unmute codec in codec driver when it loaded .
> 
> Signed-off-by: bo liu <bo.liu@...arytech.com>
> ---
>  sound/pci/hda/patch_conexant.c | 35 ++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c
> index e851785ff058..62c53e64bcad 100644
> --- a/sound/pci/hda/patch_conexant.c
> +++ b/sound/pci/hda/patch_conexant.c
> @@ -42,6 +42,7 @@ struct conexant_spec {
>  	unsigned int gpio_led;
>  	unsigned int gpio_mute_led_mask;
>  	unsigned int gpio_mic_led_mask;
> +	unsigned int gpio_unmute_bit_mask;
>  	bool is_cx8070_sn6140;
>  };
>  
> @@ -308,6 +309,7 @@ enum {
>  	CXT_FIXUP_HP_MIC_NO_PRESENCE,
>  	CXT_PINCFG_SWS_JS201D,
>  	CXT_PINCFG_TOP_SPEAKER,
> +	CXT_FIXUP_HP_A_U,
>  };
>  
>  /* for hda_fixup_thinkpad_acpi() */
> @@ -762,6 +764,24 @@ static void cxt_setup_mute_led(struct hda_codec *codec,
>  	}
>  }
>  
> +static void cxt_setup_mute_gpio_and_unmute(struct hda_codec *codec,
> +				unsigned int gpio_mute_mask)
> +{
> +	struct conexant_spec *spec = codec->spec;
> +
> +	if (gpio_mute_mask) {
> +		spec->gpio_unmute_bit_mask = gpio_mute_mask;

Any reason to store this in spec?  As far as I see the code below,
it's used only locally here, so it doesn't have to be stored there.


> +
> +		//set gpio data to 0.

Put a space after "//".

> +		snd_hda_codec_write(codec, 0x01, 0, AC_VERB_SET_GPIO_DATA, 0);
> +		snd_hda_codec_write(codec, 0x01, 0, AC_VERB_SET_GPIO_MASK,
> +					spec->gpio_unmute_bit_mask);
> +		snd_hda_codec_write(codec, 0x01, 0, AC_VERB_SET_GPIO_DIRECTION,
> +					spec->gpio_unmute_bit_mask);
> +		snd_hda_codec_write(codec, 0x01, 0, AC_VERB_SET_GPIO_STICKY_MASK, 0);
> +	}
> +}
> +
>  static void cxt_fixup_mute_led_gpio(struct hda_codec *codec,
>  				const struct hda_fixup *fix, int action)
>  {
> @@ -776,6 +796,15 @@ static void cxt_fixup_hp_zbook_mute_led(struct hda_codec *codec,
>  		cxt_setup_mute_led(codec, 0x10, 0x20);
>  }
>  
> +static void cxt_fixup_hp_a_u(struct hda_codec *codec,
> +			const struct hda_fixup *fix, int action)
> +{
> +	//Init vers in BIOS mute the spk/hp by set gpio high to avoid pop noise,
> +	//so need to unmute once by clearing the gpio data when runs into the system.

Ditto.


thanks,

Takashi