[alsa-devel] [PATCH] Cirrus Logic CS421x support

[Takashi Iwai]

At Fri, 22 Jul 2011 16:41:00 -0500,
Tim Howe wrote:
> 
> This update includes the changes necessary for supporting the
> CS421x family of codecs.  Previously this file only supported
> the CS420x family of codecs.
> 
> This file also contains init verbs to correct several issues in
> the CS421x hardware.
> 
> Behavior between the CS421x and CS420x codec families is similar,
> so several functions have been reused with "if" statements to
> determine which codec family (CS421x or CS420x) is present.
> 
> Also, this file will be updated sometime in the near future in
> order to add support for a system using CS421x that requires
> mono mix on the speaker output only.
> 
> Signed-off-by: Tim Howe <tim.howe at cirrus.com>

The patch looks mostly good.  Just a few minor points below.


> @@ -343,18 +377,44 @@ static hda_nid_t get_adc(struct hda_codec *codec, hda_nid_t pin,
>  {
>  	int i;
>  	hda_nid_t nid;
> +	struct cs_spec *spec = codec->spec;
>  
>  	nid = codec->start_nid;
>  	for (i = 0; i < codec->num_nodes; i++, nid++) {
>  		unsigned int type;
> -		int idx;
> -		type = get_wcaps_type(get_wcaps(codec, nid));
> -		if (type != AC_WID_AUD_IN)
> -			continue;
> -		idx = snd_hda_get_conn_index(codec, nid, pin, 0);
> -		if (idx >= 0) {
> -			*idxp = idx;
> -			return nid;
> +		int j, nums;
> +		/* specific to CS421x */
> +		if (spec->vendor_nid == CS421X_VENDOR_NID) {
> +			hda_nid_t pins[HDA_MAX_NUM_INPUTS];
> +			type = (get_wcaps(codec, nid) & AC_WCAP_TYPE)
> +			>> AC_WCAP_TYPE_SHIFT;

This can be simplified by get_wcaps_type(get_wcaps(codec, nid)).

> +			if (type != AC_WID_AUD_IN)
> +				continue;
> +			nums = snd_hda_get_connections(codec, nid, pins,
> +							ARRAY_SIZE(pins));
> +			if (nums <= 0)
> +				continue;
> +			for (j = 0; j < nums; j++) {
> +				if (pins[j] == pin) {
> +					*idxp = j;
> +					return nid;
> +				}
> +			}

Now we have a more easier helper function, snd_hda_get_conn_index().
The the code above will be
			*idxp = snd_hda_get_conn_index(codec, nid, pin);
			if (*idxp < 0)
				continue;
			return nid;

> @@ -1173,7 +1317,7 @@ struct cs_pincfg {
>  	u32 val;
>  };
>  
> -static const struct cs_pincfg mbp53_pincfgs[] = {
> +static struct cs_pincfg mbp53_pincfgs[] = {

Don't drop const.

> -static const struct cs_pincfg mbp55_pincfgs[] = {
> +static struct cs_pincfg mbp55_pincfgs[] = {

Ditto.

> -static const struct cs_pincfg imac27_pincfgs[] = {
> +static struct cs_pincfg imac27_pincfgs[] = {
...
> -static const struct cs_pincfg *cs_pincfgs[CS420X_MODELS] = {
> +static struct cs_pincfg *cs_pincfgs[CS420X_MODELS] = {

Keep const.

> -static void fix_pincfg(struct hda_codec *codec, int model)
> +static void fix_pincfg(struct hda_codec *codec, int model,
> +	struct cs_pincfg **pin_configs)

Put const here, too, so that you don't need to drop const.

> +/* CS4210 board pinconfigs */
> +/* Default CS4210 (CDB4210)*/
> +static struct cs_pincfg cdb4210_pincfgs[] = {

Put const.

> +static struct cs_pincfg *cs421x_pincfgs[CS421X_MODELS] = {
> +	[CS421X_CDB4210] = cdb4210_pincfgs,
> +};

Put const.

> +static struct hda_verb cs421x_coef_init_verbs[] = {

Put const.

> +static struct hda_verb cs421x_coef_init_verbs_A1_silicon_fixes[] = {

Ditto.


thanks,

Takashi