Re: [ALSA - patch_sigmatel.c for HP dv4-1222nr laptop 9/9Patch to patch_sigmatel.c for HP dv4-1222nr laptop

[Takashi Iwai <tiwai@suse.de>]

At Mon, 13 Apr 2009 18:04:16 -0400,
renidragsemaj@yahoo.com wrote:
> 
> From: James Gardiner <renidragsemaj@yahoo.com>
> 
> This patch to patch_sigmatel.c is for the HP dv4-1222nr laptop. It
> activates the included microphone(s) and the mute status change light
> for this computer. I don't know if this will work for any other dv4
> laptop. The commit isn't perfect, for it doesn't seem to work too well
> with pulseaudio - the mic boost doesn't work with that audioserver.
> Otherwise, it works well with esound, even across reboots and
> suspend/hibernate/resume. The built-in microphone sits on node 0x18 and
> feeds through node 0x1c. The gpio for the mute light is GPIO #0. I
> borrowed the code from the HP HDX. Occasionally if you press the mute
> button too fast in succession it lowers the actual volume to 0 and
> you'll have to adjust it again...But if you are doing that you probably
> are getting a kick out of making me feel bad about the behavior. :) No
> autodetect of the laptop seems to work for me, but "model=hp-dv4-1222nr"
> as an option to modprobe will make the changes.
> 
> Signed-off-by: James Gardiner <renidragsemaj@yahoo.com>

Thanks for the patch.  The change looks almost good to me.
See some comments below.

> diff --git a/sound/pci/hda/patch_sigmatel.c b/sound/pci/hda/patch_sigmatel.c
> index 61996a2..4286155 100644
...
> +static int stac92xx_hp_dv4_1222nr_check_power_status(struct hda_codec *codec,
> +						     hda_nid_t nid)
> +{
> +  /* Shamelessly stolen from the function stac92xx_hp_hdx_check_power_status */
> +  /* by James Gardiner */

In such a case, it'd be better to share the same function.
Either check spec->board_config to choose the bit to change, or add
some new fields in struct sigmtael_spec.

>  static int stac92xx_suspend(struct hda_codec *codec, pm_message_t state)
>  {
>  	struct sigmatel_spec *spec = codec->spec;
> @@ -5202,6 +5226,19 @@ again:
>  		spec->num_smuxes = 0;
>  		spec->num_dmuxes = 1;
>  		break;
> +	case STAC_HP_DV4_1222NR: /* James Gardiner */

No need to add you name here and there unless it's really needed :)
All changes are recorded via git properly, thus users can see easily
who changed what.

> +		spec->num_dmics = 1;
> +		spec->num_dmuxes = 2;
> +		stac92xx_auto_set_pinctl(codec, 0x1c, 2);

The number "2" doesn't look cute... 

Could you fix and repost the patch?


thanks,

Takashi