[PATCH] Add mixer controls to intel8x0m

[PATCH] Add mixer controls to intel8x0m

[Jaime Lopez]

This patch adds a mixer switch to the intel8x0m driver, so that the
hook state can be controlled from userspace, instead of bringing the
line off hook on capture start. Please someone test, and merge into
cvs. Comments are welcome


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click

Re: [PATCH] Add mixer controls to intel8x0m

[Jaime Lopez]

[-- Attachment #1: Type: text/plain, Size: 354 bytes --]

oops. forgot to include the diff:



On Tue, 22 Feb 2005 17:08:26 -0600, Jaime Lopez <jsollano@gmail.com> wrote:
> This patch adds a mixer switch to the intel8x0m driver, so that the
> hook state can be controlled from userspace, instead of bringing the
> line off hook on capture start. Please someone test, and merge into
> cvs. Comments are welcome
>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: intel8x0m.patch --]
[-- Type: text/x-patch; name="intel8x0m.patch", Size: 3444 bytes --]

Index: intel8x0m.c
===================================================================
RCS file: /cvsroot/alsa/alsa-kernel/pci/intel8x0m.c,v
retrieving revision 1.28
diff -r1.28 intel8x0m.c
36a37
> #include <sound/core.h>
37a39
> #include <sound/control.h>
283a286,300
> static int snd_intel8x0m_switch_default_get(snd_kcontrol_t *kcontrol,
> 			snd_ctl_elem_value_t *ucontrol);
> static int snd_intel8x0m_switch_default_put(snd_kcontrol_t *kcontrol,
> 						snd_ctl_elem_value_t *ucontrol);
> static int snd_intel8x0m_switch_default_info(snd_kcontrol_t *kcontrol,
> 						snd_ctl_elem_info_t *uinfo);
> 
> static snd_kcontrol_new_t snd_intel8x0m_mixer_switches[] __devinitdata = {
>   { .name  = "Off-hook Switch",
>     .iface = SNDRV_CTL_ELEM_IFACE_MIXER,
>     .info  = snd_intel8x0m_switch_default_info,
>     .get   = snd_intel8x0m_switch_default_get,
>     .put   = snd_intel8x0m_switch_default_put
>   }
> };
286a304,339
> static int snd_intel8x0m_switch_default_info(snd_kcontrol_t *kcontrol,
> 						snd_ctl_elem_info_t *uinfo)
> {
> 	uinfo->type = SNDRV_CTL_ELEM_TYPE_BOOLEAN;
> 	uinfo->count = 1;
> 	uinfo->value.integer.min = 0;
> 	uinfo->value.integer.max = 1;
> 	return 0;
> }
> 
> static int snd_intel8x0m_switch_default_get(snd_kcontrol_t *kcontrol,
> 			snd_ctl_elem_value_t *ucontrol)
> {
> 	intel8x0_t *chip = snd_kcontrol_chip(kcontrol);
> 	unsigned int status;
> 	spin_lock_irq(&chip->reg_lock);
> 	status = snd_ac97_read(chip->ac97,AC97_GPIO_STATUS)&AC97_GPIO_LINE1_OH?1:0;
> 	spin_unlock_irq(&chip->reg_lock);
> 	ucontrol->value.integer.value[0] = status;
> 	return 0;
> }
> static int snd_intel8x0m_switch_default_put(snd_kcontrol_t *kcontrol,
> 						snd_ctl_elem_value_t *ucontrol)
> {
> 	intel8x0_t *chip = snd_kcontrol_chip(kcontrol);
> 	unsigned short new_status = ucontrol->value.integer.value[0]?AC97_GPIO_LINE1_OH:~AC97_GPIO_LINE1_OH;
> 	int ret;
> 	//unsigned short status = snd_ac97_read(chip->ac97,AC97_GPIO_STATUS)&AC97_GPIO_LINE1_OH;
> 	spin_lock_irq(&chip->reg_lock);
> 	snd_printk("Writing status: %04x\n",new_status);
> 	ret = snd_ac97_update_bits(chip->ac97,AC97_GPIO_STATUS,
> 					AC97_GPIO_LINE1_OH, new_status);
> 	spin_unlock_irq(&chip->reg_lock);
> 	return ret;
> 	//return 0;
> }
544a598
> 	unsigned int ring = 0;
566a621,624
> 
> 	//status = igetdword(chip,ICHREG(GLOB_STA));
> 	//ring = ((status=snd_ac97_read(chip->ac97,AC97_GPIO_STATUS))&(1<<5))?1:0;
> 	//if(status != 0xffff) snd_printk("s: %08x\n",status);
640a699
> 	unsigned short regv;
646,647c705,707
< 		snd_ac97_update_bits(ichdev->ac97, AC97_GPIO_STATUS,
< 				     AC97_GPIO_LINE1_OH, AC97_GPIO_LINE1_OH);
---
> 		//snd_ac97_update_bits(ichdev->ac97, AC97_GPIO_STATUS,
> 		//		     AC97_GPIO_LINE1_OH, AC97_GPIO_LINE1_OH);
> 		//snd_ac97_update_bits(ichdev->ac97,AC97_GPIO_STATUS,0x40,0x40);
650,651c710,712
< 		snd_ac97_update_bits(ichdev->ac97, AC97_GPIO_STATUS,
< 				     AC97_GPIO_LINE1_OH, ~AC97_GPIO_LINE1_OH);
---
> 		//snd_ac97_update_bits(ichdev->ac97, AC97_GPIO_STATUS,
> 		//		     AC97_GPIO_LINE1_OH, ~AC97_GPIO_LINE1_OH);
> 		//snd_ac97_update_bits(ichdev->ac97,AC97_GPIO_STATUS,0x40,~0x40);
892a954
> 	unsigned int idx;
927a990,993
> 	for(idx = 0; idx < ARRAY_SIZE(snd_intel8x0m_mixer_switches); idx++) {
> 		if((err = snd_ctl_add(chip->card,snd_ctl_new1(&snd_intel8x0m_mixer_switches[idx],chip))) <0 )
> 		goto __err;
> 	}

Re: Re: [PATCH] Add mixer controls to intel8x0m

[Takashi Iwai]

At Tue, 22 Feb 2005 17:14:01 -0600,
Jaime Lopez wrote:
> 
> oops. forgot to include the diff:

Please create a unified style patch with diff -u.
Also, don't forget to provide "Signed-off-by" line, too, such as

	Signed-off-by: Foo Bar <foo@bar.com>

to add your credit to changelog.


The patch looks almost OK to me (if it works :)

Some points I noticed, though:

- The callback function names can be other ones that match better to
  the control itself
- Remove spin_lock_irq() around snd_ac97_*() calls
- Clean up the comments


thanks,

Takashi


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click

Re: Re: [PATCH] Add mixer controls to intel8x0m

[Jaime Lopez]

[-- Attachment #1: Type: text/plain, Size: 416 bytes --]

> Please create a unified style patch with diff -u.

here it is

> - The callback function names can be other ones that match better to
>   the control itself

I decided to keep the names but made the callbacks reusable. Hope you
don't mind.

> - Remove spin_lock_irq() around snd_ac97_*() calls
done
> - Clean up the comments
> 
> thanks,
> 
> Takashi
> 

Signed-off-by: Jaime A. Lopez Sollano <jsollano@gmail.com>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: intel8x0m.c.patch --]
[-- Type: text/x-patch; name="intel8x0m.c.patch", Size: 3948 bytes --]

Index: intel8x0m.c
===================================================================
RCS file: /cvsroot/alsa/alsa-kernel/pci/intel8x0m.c,v
retrieving revision 1.28
diff -u -r1.28 intel8x0m.c
--- intel8x0m.c	17 Feb 2005 14:48:07 -0000	1.28
+++ intel8x0m.c	23 Feb 2005 15:30:10 -0000
@@ -34,7 +34,9 @@
 #include <sound/core.h>
 #include <sound/pcm.h>
 #include <sound/ac97_codec.h>
+#include <sound/core.h>
 #include <sound/info.h>
+#include <sound/control.h>
 #include <sound/initval.h>
 
 MODULE_AUTHOR("Jaroslav Kysela <perex@suse.cz>");
@@ -281,9 +283,63 @@
 #endif
 	{ 0, }
 };
+static int snd_intel8x0m_switch_default_get(snd_kcontrol_t *kcontrol,
+			snd_ctl_elem_value_t *ucontrol);
+static int snd_intel8x0m_switch_default_put(snd_kcontrol_t *kcontrol,
+						snd_ctl_elem_value_t *ucontrol);
+static int snd_intel8x0m_switch_default_info(snd_kcontrol_t *kcontrol,
+						snd_ctl_elem_info_t *uinfo);
+struct snd_intel8x0m_mixer_switch_args {
+	unsigned short reg;
+	unsigned short mask;
+};
+#define PRIVATE_VALUE_INITIALIZER(r,m) \
+(unsigned long)&(struct snd_intel8x0m_mixer_switch_args) \
+{ .reg = r, .mask = m}
+
+static snd_kcontrol_new_t snd_intel8x0m_mixer_switches[] __devinitdata = {
+  { .name  = "Off-hook Switch",
+    .iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+    .info  = snd_intel8x0m_switch_default_info,
+    .get   = snd_intel8x0m_switch_default_get,
+    .put   = snd_intel8x0m_switch_default_put,
+    .private_value = PRIVATE_VALUE_INITIALIZER(AC97_GPIO_STATUS,AC97_GPIO_LINE1_OH)
+  }
+};
 
 MODULE_DEVICE_TABLE(pci, snd_intel8x0m_ids);
 
+static int snd_intel8x0m_switch_default_info(snd_kcontrol_t *kcontrol,
+						snd_ctl_elem_info_t *uinfo)
+{
+	uinfo->type = SNDRV_CTL_ELEM_TYPE_BOOLEAN;
+	uinfo->count = 1;
+	uinfo->value.integer.min = 0;
+	uinfo->value.integer.max = 1;
+	return 0;
+}
+
+static int snd_intel8x0m_switch_default_get(snd_kcontrol_t *kcontrol,
+			snd_ctl_elem_value_t *ucontrol)
+{
+	struct snd_intel8x0m_mixer_switch_args *args = (void *)kcontrol->private_value;
+	intel8x0_t *chip = snd_kcontrol_chip(kcontrol);
+	unsigned int status;
+	status = snd_ac97_read(chip->ac97,args->reg)&args->mask?1:0;
+	ucontrol->value.integer.value[0] = status;
+	return 0;
+}
+static int snd_intel8x0m_switch_default_put(snd_kcontrol_t *kcontrol,
+						snd_ctl_elem_value_t *ucontrol)
+{
+	struct snd_intel8x0m_mixer_switch_args *args = (void *)kcontrol->private_value;
+	intel8x0_t *chip = snd_kcontrol_chip(kcontrol);
+	unsigned short new_status = ucontrol->value.integer.value[0]?args->mask:~args->mask;
+	int ret;
+	ret = snd_ac97_update_bits(chip->ac97,args->reg,
+					args->mask, new_status);
+	return ret;
+}
 /*
  *  Lowlevel I/O - busmaster
  */
@@ -638,17 +694,12 @@
 
 static int snd_intel8x0m_pcm_trigger(snd_pcm_substream_t *substream, int cmd)
 {
-	ichdev_t *ichdev = get_ichdev(substream);
 	/* hook off/on on start/stop */
-	/* TODO: move it to ac97 controls */
+	/* Moved this to mixer control */
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
-		snd_ac97_update_bits(ichdev->ac97, AC97_GPIO_STATUS,
-				     AC97_GPIO_LINE1_OH, AC97_GPIO_LINE1_OH);
 		break;
 	case SNDRV_PCM_TRIGGER_STOP:
-		snd_ac97_update_bits(ichdev->ac97, AC97_GPIO_STATUS,
-				     AC97_GPIO_LINE1_OH, ~AC97_GPIO_LINE1_OH);
 		break;
 	default:
 		return -EINVAL;
@@ -890,6 +941,7 @@
 	ac97_t *x97;
 	int err;
 	unsigned int glob_sta = 0;
+	unsigned int idx;
 	static ac97_bus_ops_t ops = {
 		.write = snd_intel8x0_codec_write,
 		.read = snd_intel8x0_codec_read,
@@ -925,6 +977,10 @@
 		chip->ichd[ICHD_MDMIN].ac97 = x97;
 		chip->ichd[ICHD_MDMOUT].ac97 = x97;
 	}
+	for(idx = 0; idx < ARRAY_SIZE(snd_intel8x0m_mixer_switches); idx++) {
+		if((err = snd_ctl_add(chip->card,snd_ctl_new1(&snd_intel8x0m_mixer_switches[idx],chip))) <0 )
+		goto __err;
+	}
 
 	chip->in_ac97_init = 0;
 	return 0;