Hi,
sorry for the late review. Some comments below.
At Wed, 20 May 2009 12:35:41 +0000,
Nickolas Lloyd wrote:
>
> +static int stac92xx_dc_bias_info(struct snd_kcontrol *kcontrol,
> + struct snd_ctl_elem_info *uinfo)
> +{
> + int i;
> + static char *texts[] = {
> + "Mic In", "Line In", "Line Out"
> + };
> +
> + if (kcontrol->private_value & 0x100)
Please avoid a magic number. Define a constant if needed.
> + i = 3;
> + else
> + i = 2;
> +
> + uinfo->type = SNDRV_CTL_ELEM_TYPE_ENUMERATED;
> + uinfo->value.enumerated.items = i;
> + uinfo->count = 1;
> + if (uinfo->value.enumerated.item >= i)
> + uinfo->value.enumerated.item = i-1;
Didn't checkpatch.pl complain? :)
> +static int stac92xx_dc_bias_get(struct snd_kcontrol *kcontrol,
> + struct snd_ctl_elem_value *ucontrol)
> +{
> + struct hda_codec *codec = snd_kcontrol_chip(kcontrol);
> + hda_nid_t nid = kcontrol->private_value & 0x0ff;
Better to define a macro to retrieve a value.
> + unsigned int vref = stac92xx_vref_get(codec, nid);
> +
> + if (vref == stac92xx_get_vref(codec, nid))
Well, these two functions, stac92xx_vref_get() and stac92xx_get_vref()
are too confusing. We have to rename one of them, at least, or at best
clean up a bit more.
> @@ -3261,18 +3294,34 @@ static int stac92xx_auto_create_multi_ou
>
> if (spec->line_switch) {
> err = stac92xx_add_control(spec, STAC_CTL_WIDGET_IO_SWITCH,
> - "Line In as Output Switch",
> + "Line In Jack Mode",
> spec->line_switch << 8);
> if (err < 0)
> return err;
> }
>
> - if (spec->mic_switch) {
> - err = stac92xx_add_control(spec, STAC_CTL_WIDGET_DC_BIAS,
> - "Mic Jack Mode",
> - spec->mic_switch);
> - if (err < 0)
> - return err;
> + nid = cfg->input_pins[AUTO_PIN_MIC];
> + idx = 0;
> +again:
> + if (nid) {
> + def_conf = snd_hda_codec_get_pincfg(codec, nid);
> + if (!((get_defcfg_connect(def_conf)) & AC_JACK_PORT_FIXED)) {
> + if (snd_hda_query_pin_caps(codec, nid)
> + & (AC_PINCAP_VREF_GRD << AC_PINCAP_VREF_SHIFT))
> + stac92xx_add_control_idx(spec,
> + STAC_CTL_WIDGET_DC_BIAS, idx++,
> + "Mic Jack Mode", nid
> + | ((nid == spec->mic_switch)
> + ? 0x100 : 0));
> + else if (nid == spec->mic_switch)
> + stac92xx_add_control_idx(spec,
> + STAC_CTL_WIDGET_IO_SWITCH, idx++,
> + "Mic Jack Mode", ((nid << 8) | 1));
> + }
> + }
> + if (nid == cfg->input_pins[AUTO_PIN_MIC]) {
> + nid = cfg->input_pins[AUTO_PIN_FRONT_MIC];
> + goto again;
> }
Hmm, this code flow is too complex and hackish to follow.
There must be a better way. For example, the complicated neted if
checks can be put out to a function. The compiler is clever enough to
inline, and here is no critical code path for speed.
Could you fix and repost please?
thanks,
Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
