Re: [PATCH V4] Initialize the gain and boost values of the ADC in the SN6180 chip to achieve a better recording experience.

[Takashi Iwai <tiwai@suse.de>]

On Tue, 25 Mar 2025 07:09:56 +0100,
bo liu wrote:
> 
> Signed-off-by: bo liu <bo.liu@senarytech.com>

First off, could you use a proper subject line?  It should be more
concise, and have a prefix such as "ALSA: hda/conexant:".

Then put more information in the patch description, instead of putting
to the subject line.

About the code change:
>  
> +static void cxt_setup_capture_default_boost_gain(struct hda_codec *codec)
> +{

Better to give more comments about what this function actually does.

> +	struct conexant_spec *spec = codec->spec;
> +	struct hda_gen_spec *gen = &(spec->gen);

No need for parentheses.

> +	hda_nid_t adc_id = 0, nid = 0;

Why initializing those?

> +	int nids;

This should be either u32 or unsigned int.

> +	size_t i = 0;

It's superfluous initialization.

> +
> +	for (i = 0; i < gen->num_all_adcs; i++) {
> +		int index;
> +
> +		adc_id = gen->all_adcs[i];
> +
> +		index = snd_hda_codec_read(codec, adc_id, 0, AC_VERB_GET_CONNECT_SEL, 0);
> +		nids = snd_hda_codec_read(codec, adc_id, 0, AC_VERB_GET_CONNECT_LIST, 0);
> +		nid = ((nids & (0xFF << (8*index))) >> (8*index));

It's simpler and clearer to do right-shift at first, then mask.

> +		if (nid < codec->core.start_nid || nid > codec->core.end_nid) {
> +			codec_err(codec, "Error, invalid nid 0x%02x.\n", nid);
> +			continue;
> +		}
> +
> +		snd_hda_codec_write(codec, adc_id, 0, (0x370|(index&0xF)), 0x4A);

Too cryptic.  Use the defined verbs.

> +		snd_hda_codec_write(codec, nid, 0, 0x370, 0x2);

Ditto.

> +	}
> +}
> +
> +static void cxt_init_capture_boost_gain(struct hda_codec *codec)
> +{
> +	cxt_setup_capture_default_boost_gain(codec);
> +}

Why nested calls...?


thanks,

Takashi