Re: [alsa-devel] [PATCH] HDA - add "Independent HP" switch for ad1988

At Sun, 18 Sep 2011 07:18:28 +0800, Raymond Yau wrote:

Add "Independent HP" switch for ad1988

• add playback device 2 "AD1988 HP" for 7.1+2 multi streaming playback
• add "Independent HP" switch to enable/disable the feature switch is inactive and write protect when device 0 or device 2 is opened
• remove 6stack-dig-fp model

Any rationale to remove this model? I'm fine with the removal, but need to know the reason.

Also, regarding the patch:

@@ -302,6 +304,71 @@ static int ad198x_check_power_status(struct hda_codec *codec, hda_nid_t nid) } #endif

+static void activate_ctl(struct hda_codec *codec, const char *name, int active) +{

• struct snd_kcontrol *ctl = snd_hda_find_mixer_ctl(codec, name);
• if (ctl) {

• ctl->vd[0].access &= ~SNDRV_CTL_ELEM_ACCESS_INACTIVE;
  

• ctl->vd[0].access |= active ? 0:SNDRV_CTL_ELEM_ACCESS_INACTIVE;
  

•            ctl->vd[0].access &= ~SNDRV_CTL_ELEM_ACCESS_WRITE;
  

Please use tabs.

• ctl->vd[0].access |= active ? SNDRV_CTL_ELEM_ACCESS_WRITE:0;       
  

• snd_ctl_notify(codec->bus->card,
  

• 	       SNDRV_CTL_EVENT_MASK_INFO, &ctl->id);
  

• }

+}

• static void set_stream_active(struct hda_codec *codec, bool active)

+{

• struct ad198x_spec *spec = codec->spec;
• if (active)

• spec->num_active_streams++;
  

• else

• spec->num_active_streams--;
  

• activate_ctl(codec, "Independent HP", spec->num_active_streams == 0);
• printk(KERN_INFO "set stream active : active stream %d \n", spec->num_active_streams);

Avoid debug prints in the final code. If any, use snd_printdd() or such.

+}

+ +static int ad1988_independent_hp_info(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_info *uinfo) +{ + static const char * const texts[] = { "OFF", "ON", NULL}; + int index; + uinfo->type = SNDRV_CTL_ELEM_TYPE_ENUMERATED; + uinfo->count = 1; + uinfo->value.enumerated.items = 2; + index = uinfo->value.enumerated.item; + if (index >= 2) + index = 1; + strcpy(uinfo->value.enumerated.name, texts[index]); + return 0; +} + +static int ad1988_independent_hp_get(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct hda_codec *codec = snd_kcontrol_chip(kcontrol); + struct ad198x_spec *spec = codec->spec; + ucontrol->value.enumerated.item[0] = spec->independent_hp; + return 0; +} + +static int ad1988_independent_hp_put(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct hda_codec *codec = snd_kcontrol_chip(kcontrol); + struct ad198x_spec *spec = codec->spec; + unsigned int select = ucontrol->value.enumerated.item[0]; + if (spec->independent_hp != select) { + spec->independent_hp = select; + if (spec->independent_hp) + spec->multiout.hp_nid = 0; + else + spec->multiout.hp_nid = spec->alt_dac_nid[0]; + return 1; + } + return 0; +}

Changing spec->multiout.hp_nid dynamically here is racy. If the value is changed during the PCM stream is opened, the HP-setup might be kept inconsistent at close.

thanks,

Takashi