[es18xx.c PATCH] #1/4 for Zoom Video - resolve common vs chipset specific mixer controls

[es18xx.c PATCH] #1/4 for Zoom Video - resolve common vs chipset specific mixer controls

[Mark Salazar]

First of 4 es18xx.c patches culminating in Zoom Video support.
While adding support for Zoom Video to the es18xx driver I found some of 
the mixer controls
were wrong. Since you guys went to the trouble of supplying the 
datasheets for the supported
chipsets I did a review of all of them and tried to get es18xx.c to 
accurately reflect the
proper mixer controls for each chipset. If the datasheets are wrong then 
so are my patches.

This first patch moves some controls from the common-to-all-chipsets array
'snd_es18xx_base_controls' to a chipset-specific array and adds code to 
manage that new array.
Also while testing on my ES1878 test machine I discovered it needed a 
couple of udelays in
the identify function so those are in this patch as well.

Testing:
This work was initially done on the source from the Debian Sarge ALSA 
package, then tested
on an ES1879 and an ES1878 machine. Patches were created against the 
Sarge code and then edited
to apply correctly to the ALSA cvs code. Lastly the patched ALSA cvs 
code was test for
successful compilation. No additional testing was done on the ALSA cvs 
version.

Applying:
cd alsa-driver/
cat ../../es18xx.CVSmod1.diff |  patch -p1

Signed-off-by: Mark Salazar <markTheCoder@justmyself.net>

------
diff -Nur ../alsa-driver/alsa-kernel/isa/es18xx.c ./alsa-kernel/isa/es18xx.c
--- ../alsa-driver/alsa-kernel/isa/es18xx.c    2004-12-15 
10:26:10.000000000 -0500
+++ ./alsa-kernel/isa/es18xx.c    2005-12-10 13:59:30.000000000 -0500
@@ -1181,19 +1181,22 @@
     return change;
 }
 
+/* Mixer controls
+ * These arrays contain setup data for mixer controls.
+ *
+ * The controls that are universal to all chipsets are fully initialized
+ * here.
+ */
 static struct snd_kcontrol_new snd_es18xx_base_controls[] = {
 ES18XX_DOUBLE("Master Playback Volume", 0, 0x60, 0x62, 0, 0, 63, 0),
 ES18XX_DOUBLE("Master Playback Switch", 0, 0x60, 0x62, 6, 6, 1, 1),
 ES18XX_DOUBLE("Line Playback Volume", 0, 0x3e, 0x3e, 4, 0, 15, 0),
 ES18XX_DOUBLE("CD Playback Volume", 0, 0x38, 0x38, 4, 0, 15, 0),
 ES18XX_DOUBLE("FM Playback Volume", 0, 0x36, 0x36, 4, 0, 15, 0),
-ES18XX_DOUBLE("Mono Playback Volume", 0, 0x6d, 0x6d, 4, 0, 15, 0),
 ES18XX_DOUBLE("Mic Playback Volume", 0, 0x1a, 0x1a, 4, 0, 15, 0),
 ES18XX_DOUBLE("Aux Playback Volume", 0, 0x3a, 0x3a, 4, 0, 15, 0),
-ES18XX_SINGLE("PC Speaker Playback Volume", 0, 0x3c, 0, 7, 0),
 ES18XX_SINGLE("Record Monitor", 0, 0xa8, 3, 1, 0),
 ES18XX_DOUBLE("Capture Volume", 0, 0xb4, 0xb4, 4, 0, 15, 0),
-ES18XX_SINGLE("Capture Switch", 0, 0x1c, 4, 1, 1),
 {
     .iface = SNDRV_CTL_ELEM_IFACE_MIXER,
     .name = "Capture Source",
@@ -1203,19 +1206,34 @@
 }
 };
 
-static struct snd_kcontrol_new snd_es18xx_mono_in_control =
-ES18XX_DOUBLE("Mono Input Playback Volume", 0, 0x6d, 0x6d, 4, 0, 15, 0);
-
 static struct snd_kcontrol_new snd_es18xx_recmix_controls[] = {
 ES18XX_DOUBLE("PCM Capture Volume", 0, 0x69, 0x69, 4, 0, 15, 0),
 ES18XX_DOUBLE("Mic Capture Volume", 0, 0x68, 0x68, 4, 0, 15, 0),
 ES18XX_DOUBLE("Line Capture Volume", 0, 0x6e, 0x6e, 4, 0, 15, 0),
 ES18XX_DOUBLE("FM Capture Volume", 0, 0x6b, 0x6b, 4, 0, 15, 0),
-ES18XX_DOUBLE("Mono Capture Volume", 0, 0x6f, 0x6f, 4, 0, 15, 0),
 ES18XX_DOUBLE("CD Capture Volume", 0, 0x6a, 0x6a, 4, 0, 15, 0),
 ES18XX_DOUBLE("Aux Capture Volume", 0, 0x6c, 0x6c, 4, 0, 15, 0)
 };
 
+/*
+ * These chipset specific mixer controls are initialized in
+ * snd_es18xx_mixer. Those controls which are not UNSET are then
+ * instantiated.
+ */
+static char UNSETbuf = '\0';
+#define UNSET ((char *)&UNSETbuf)
+static enum optionalControlsIndex {reg1cBit4, reg3cMono, reg6d, reg6f};
+static struct snd_kcontrol_new snd_es18xx_optional_controls[] = {
+ES18XX_SINGLE(UNSET, 0, 0x1c, 4, 1, 1),
+ES18XX_SINGLE(UNSET, 0, 0x3c, 0, 7, 0),
+ES18XX_DOUBLE(UNSET, 0, 0x6d, 0x6d, 4, 0, 15, 0),
+ES18XX_DOUBLE(UNSET, 0, 0x6f, 0x6f, 4, 0, 15, 0)
+};
+static char* captureSwitchName = "Capture Switch";
+static char* speakerPlayName = "PC Speaker Playback Volume";
+static char* monoPlayName = "Mono Playback Volume";
+static char* monoRecName = "Mono Capture Volume";
+
 static struct snd_kcontrol_new snd_es18xx_pcm1_controls[] = {
 ES18XX_DOUBLE("PCM Playback Volume", 0, 0x14, 0x14, 4, 0, 15, 0),
 };
@@ -1466,11 +1484,14 @@
     }
             
         outb(0x40, chip->port + 0x04);
+    udelay(10);
     hi = inb(chip->port + 0x05);
+    udelay(10);
     lo = inb(chip->port + 0x05);
     if (hi != lo) {
         chip->version = hi << 8 | lo;
         chip->ctrl_port = inb(chip->port + 0x05) << 8;
+        udelay(10);
         chip->ctrl_port += inb(chip->port + 0x05);
 
         if ((chip->res_ctrl_port = request_region(chip->ctrl_port, 8, 
"ES18xx - CTRL")) == NULL) {
@@ -1781,10 +1802,6 @@
         }
     }
 
-    if (chip->caps & ES18XX_MONO) {
-        if ((err = snd_ctl_add(card, 
snd_ctl_new1(&snd_es18xx_mono_in_control, chip))) < 0)
-            return err;
-    }
     if (chip->caps & ES18XX_RECMIX) {
         for (idx = 0; idx < ARRAY_SIZE(snd_es18xx_recmix_controls); 
idx++) {
             if ((err = snd_ctl_add(card, 
snd_ctl_new1(&snd_es18xx_recmix_controls[idx], chip))) < 0)
@@ -1822,6 +1839,24 @@
             
         }
     }
+/* finish initializing other chipset specific controls
+ */
+    if (chip->version != 0x1868) {
+        snd_es18xx_optional_controls[reg3cMono].name = speakerPlayName;
+    }
+    switch (chip->version) {
+    case 0x1869:
+        snd_es18xx_optional_controls[reg1cBit4].name = captureSwitchName;
+        snd_es18xx_optional_controls[reg6d].name = monoPlayName;
+        snd_es18xx_optional_controls[reg6f].name = monoRecName;
+        break;
+    }
+    for (idx = 0; idx < ARRAY_SIZE(snd_es18xx_optional_controls); idx++) {
+        if (snd_es18xx_optional_controls[idx].name == UNSET)
+            continue;
+        if ((err = snd_ctl_add(card, 
snd_ctl_new1(&snd_es18xx_optional_controls[idx], chip))) < 0)
+            return err;
+    }
     return 0;
 }
        



-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click

Re: [es18xx.c PATCH] #1/4 for Zoom Video - resolve common vs chipset specific mixer controls

[Lee Revell]

On Mon, 2005-12-26 at 10:44 -0500, Mark Salazar wrote:
> First of 4 es18xx.c patches culminating in Zoom Video support.

All of these except the first are linewrapped and can't be applied.

Lee



-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click

Re: [es18xx.c PATCH] #1/4 for Zoom Video - resolve common vs chipset specific mixer controls

[Mark Salazar]

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

Lee Revell wrote:

>On Mon, 2005-12-26 at 10:44 -0500, Mark Salazar wrote:
>> First of 4 es18xx.c patches culminating in Zoom Video support.
>
>All of these except the first are linewrapped and can't be applied.
>  
>

I surrender. Unable to make Thunderbird's wrapping function submit to 
my will I just attach my patch message and slink away.



[-- Attachment #2: es18xx.CVSmod1.diff.msg --]
[-- Type: text/plain, Size: 5723 bytes --]

First of 4 es18xx.c patches culminating in Zoom Video support.
While adding support for Zoom Video to the es18xx driver I found some of the mixer controls
were wrong. Since you guys went to the trouble of supplying the datasheets for the supported
chipsets I did a review of all of them and tried to get es18xx.c to accurately reflect the
proper mixer controls for each chipset. If the datasheets are wrong then so are my patches.

This first patch moves some controls from the common-to-all-chipsets array 
'snd_es18xx_base_controls' to a chipset-specific array and adds code to manage that new array.
Also while testing on my ES1878 test machine I discovered it needed a couple of udelays in
the identify function so those are in this patch as well.

Testing:
This work was initially done on the source from the Debian Sarge ALSA package, then tested
on an ES1879 and an ES1878 machine. Patches were created against the Sarge code and then edited
to apply correctly to the ALSA cvs code. Lastly the patched ALSA cvs code was test for
successful compilation. No additional testing was done on the ALSA cvs version.

Applying:
cd alsa-driver/
cat ../../es18xx.CVSmod1.diff |  patch -p1

Signed-off-by: Mark Salazar <markTheCoder@justmyself.net>

------
diff -Nur ../alsa-driver/alsa-kernel/isa/es18xx.c ./alsa-kernel/isa/es18xx.c
--- ../alsa-driver/alsa-kernel/isa/es18xx.c	2004-12-15 10:26:10.000000000 -0500
+++ ./alsa-kernel/isa/es18xx.c	2005-12-10 13:59:30.000000000 -0500
@@ -1181,19 +1181,22 @@
 	return change;
 }
 
+/* Mixer controls
+ * These arrays contain setup data for mixer controls.
+ * 
+ * The controls that are universal to all chipsets are fully initialized
+ * here.
+ */
 static struct snd_kcontrol_new snd_es18xx_base_controls[] = {
 ES18XX_DOUBLE("Master Playback Volume", 0, 0x60, 0x62, 0, 0, 63, 0),
 ES18XX_DOUBLE("Master Playback Switch", 0, 0x60, 0x62, 6, 6, 1, 1),
 ES18XX_DOUBLE("Line Playback Volume", 0, 0x3e, 0x3e, 4, 0, 15, 0),
 ES18XX_DOUBLE("CD Playback Volume", 0, 0x38, 0x38, 4, 0, 15, 0),
 ES18XX_DOUBLE("FM Playback Volume", 0, 0x36, 0x36, 4, 0, 15, 0),
-ES18XX_DOUBLE("Mono Playback Volume", 0, 0x6d, 0x6d, 4, 0, 15, 0),
 ES18XX_DOUBLE("Mic Playback Volume", 0, 0x1a, 0x1a, 4, 0, 15, 0),
 ES18XX_DOUBLE("Aux Playback Volume", 0, 0x3a, 0x3a, 4, 0, 15, 0),
-ES18XX_SINGLE("PC Speaker Playback Volume", 0, 0x3c, 0, 7, 0),
 ES18XX_SINGLE("Record Monitor", 0, 0xa8, 3, 1, 0),
 ES18XX_DOUBLE("Capture Volume", 0, 0xb4, 0xb4, 4, 0, 15, 0),
-ES18XX_SINGLE("Capture Switch", 0, 0x1c, 4, 1, 1),
 {
 	.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
 	.name = "Capture Source",
@@ -1203,19 +1206,34 @@
 }
 };
 
-static struct snd_kcontrol_new snd_es18xx_mono_in_control = 
-ES18XX_DOUBLE("Mono Input Playback Volume", 0, 0x6d, 0x6d, 4, 0, 15, 0);
-
 static struct snd_kcontrol_new snd_es18xx_recmix_controls[] = {
 ES18XX_DOUBLE("PCM Capture Volume", 0, 0x69, 0x69, 4, 0, 15, 0),
 ES18XX_DOUBLE("Mic Capture Volume", 0, 0x68, 0x68, 4, 0, 15, 0),
 ES18XX_DOUBLE("Line Capture Volume", 0, 0x6e, 0x6e, 4, 0, 15, 0),
 ES18XX_DOUBLE("FM Capture Volume", 0, 0x6b, 0x6b, 4, 0, 15, 0),
-ES18XX_DOUBLE("Mono Capture Volume", 0, 0x6f, 0x6f, 4, 0, 15, 0),
 ES18XX_DOUBLE("CD Capture Volume", 0, 0x6a, 0x6a, 4, 0, 15, 0),
 ES18XX_DOUBLE("Aux Capture Volume", 0, 0x6c, 0x6c, 4, 0, 15, 0)
 };
 
+/*
+ * These chipset specific mixer controls are initialized in
+ * snd_es18xx_mixer. Those controls which are not UNSET are then
+ * instantiated.
+ */
+static char UNSETbuf = '\0';
+#define UNSET ((char *)&UNSETbuf)
+static enum optionalControlsIndex {reg1cBit4, reg3cMono, reg6d, reg6f};
+static struct snd_kcontrol_new snd_es18xx_optional_controls[] = {
+ES18XX_SINGLE(UNSET, 0, 0x1c, 4, 1, 1),
+ES18XX_SINGLE(UNSET, 0, 0x3c, 0, 7, 0),
+ES18XX_DOUBLE(UNSET, 0, 0x6d, 0x6d, 4, 0, 15, 0),
+ES18XX_DOUBLE(UNSET, 0, 0x6f, 0x6f, 4, 0, 15, 0)
+};
+static char* captureSwitchName = "Capture Switch";
+static char* speakerPlayName = "PC Speaker Playback Volume";
+static char* monoPlayName = "Mono Playback Volume";
+static char* monoRecName = "Mono Capture Volume";
+
 static struct snd_kcontrol_new snd_es18xx_pcm1_controls[] = {
 ES18XX_DOUBLE("PCM Playback Volume", 0, 0x14, 0x14, 4, 0, 15, 0),
 };
@@ -1466,11 +1484,14 @@
 	}
 			
         outb(0x40, chip->port + 0x04);
+	udelay(10);
 	hi = inb(chip->port + 0x05);
+	udelay(10);
 	lo = inb(chip->port + 0x05);
 	if (hi != lo) {
 		chip->version = hi << 8 | lo;
 		chip->ctrl_port = inb(chip->port + 0x05) << 8;
+		udelay(10);
 		chip->ctrl_port += inb(chip->port + 0x05);
 
 		if ((chip->res_ctrl_port = request_region(chip->ctrl_port, 8, "ES18xx - CTRL")) == NULL) {
@@ -1781,10 +1802,6 @@
 		}
 	}
 
-	if (chip->caps & ES18XX_MONO) {
-		if ((err = snd_ctl_add(card, snd_ctl_new1(&snd_es18xx_mono_in_control, chip))) < 0)
-			return err;
-	}
 	if (chip->caps & ES18XX_RECMIX) {
 		for (idx = 0; idx < ARRAY_SIZE(snd_es18xx_recmix_controls); idx++) {
 			if ((err = snd_ctl_add(card, snd_ctl_new1(&snd_es18xx_recmix_controls[idx], chip))) < 0)
@@ -1822,6 +1839,24 @@
 			
 		}
 	}
+/* finish initializing other chipset specific controls
+ */
+	if (chip->version != 0x1868) {
+		snd_es18xx_optional_controls[reg3cMono].name = speakerPlayName;
+	}
+	switch (chip->version) {
+	case 0x1869:
+		snd_es18xx_optional_controls[reg1cBit4].name = captureSwitchName;
+		snd_es18xx_optional_controls[reg6d].name = monoPlayName;
+		snd_es18xx_optional_controls[reg6f].name = monoRecName;
+		break;
+	}
+	for (idx = 0; idx < ARRAY_SIZE(snd_es18xx_optional_controls); idx++) {
+		if (snd_es18xx_optional_controls[idx].name == UNSET)
+			continue;
+		if ((err = snd_ctl_add(card, snd_ctl_new1(&snd_es18xx_optional_controls[idx], chip))) < 0)
+			return err;
+	}
 	return 0;
 }
        

Re: [es18xx.c PATCH] #1/4 for Zoom Video - resolve common vs chipset specific mixer controls

[Lee Revell]

On Mon, 2005-12-26 at 12:36 -0500, Mark Salazar wrote:
> Lee Revell wrote:
> 
> >On Mon, 2005-12-26 at 10:44 -0500, Mark Salazar wrote:
> >> First of 4 es18xx.c patches culminating in Zoom Video support.
> >
> >All of these except the first are linewrapped and can't be applied.
> >  
> >
> 
> I surrender. Unable to make Thunderbird's wrapping function submit to 
> my will I just attach my patch message and slink away.

Wow is Thunderbird really such garbage that this is impossible?  It
would explain why we see so many line wrapped patches on LKML.

Someone should really follow up on this with the Mozilla developers.

Lee



-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click

Re: [es18xx.c PATCH] #1/4 for Zoom Video - resolve common vs chipset specific mixer controls

[Takashi Iwai]

At Mon, 26 Dec 2005 12:36:14 -0500,
Mark Salazar wrote:
> 
> +/*
> + * These chipset specific mixer controls are initialized in
> + * snd_es18xx_mixer. Those controls which are not UNSET are then
> + * instantiated.
> + */
> +static char UNSETbuf = '\0';
> +#define UNSET ((char *)&UNSETbuf)
> +static enum optionalControlsIndex {reg1cBit4, reg3cMono, reg6d, reg6f};
> +static struct snd_kcontrol_new snd_es18xx_optional_controls[] = {
> +ES18XX_SINGLE(UNSET, 0, 0x1c, 4, 1, 1),
> +ES18XX_SINGLE(UNSET, 0, 0x3c, 0, 7, 0),
> +ES18XX_DOUBLE(UNSET, 0, 0x6d, 0x6d, 4, 0, 15, 0),
> +ES18XX_DOUBLE(UNSET, 0, 0x6f, 0x6f, 4, 0, 15, 0)
> +};
> +static char* captureSwitchName = "Capture Switch";
> +static char* speakerPlayName = "PC Speaker Playback Volume";
> +static char* monoPlayName = "Mono Playback Volume";
> +static char* monoRecName = "Mono Capture Volume";

This doesn't work for the multiple cards because you overwrite the
static array.  Better to create once kcontrol instaces via
snd_ctl_new1(), then copy the name on each instance according to the
model.

Other chnages look fine to me.

thanks,

Takashi


-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click

Re: [es18xx.c PATCH] #1/4 for Zoom Video - resolve common vs chipset specific mixer controls

[Mark Salazar]

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

Takashi Iwai wrote:

>This doesn't work for the multiple cards because you overwrite the
>static array.  
>
Yup. Good catch. Chalk up another one for peer review.

>Better to create once kcontrol instaces via
>snd_ctl_new1(), then copy the name on each instance according to the
>model.
>  
>
Hmmm .. what if I just reset it to UNSET in the loop where it's used (as 
this patch does)?

New patch attached . Tested same as original version.


[-- Attachment #2: es18xx.CVSmod1V2.diff --]
[-- Type: text/plain, Size: 4741 bytes --]

diff -Nur ../alsa-driver/alsa-kernel/isa/es18xx.c ./alsa-kernel/isa/es18xx.c
--- ../alsa-driver/alsa-kernel/isa/es18xx.c	2004-12-15 10:26:10.000000000 -0500
+++ ./alsa-kernel/isa/es18xx.c	2006-01-03 21:50:26.000000000 -0500
@@ -1181,19 +1181,22 @@
 	return change;
 }
 
+/* Mixer controls
+ * These arrays contain setup data for mixer controls.
+ * 
+ * The controls that are universal to all chipsets are fully initialized
+ * here.
+ */
 static struct snd_kcontrol_new snd_es18xx_base_controls[] = {
 ES18XX_DOUBLE("Master Playback Volume", 0, 0x60, 0x62, 0, 0, 63, 0),
 ES18XX_DOUBLE("Master Playback Switch", 0, 0x60, 0x62, 6, 6, 1, 1),
 ES18XX_DOUBLE("Line Playback Volume", 0, 0x3e, 0x3e, 4, 0, 15, 0),
 ES18XX_DOUBLE("CD Playback Volume", 0, 0x38, 0x38, 4, 0, 15, 0),
 ES18XX_DOUBLE("FM Playback Volume", 0, 0x36, 0x36, 4, 0, 15, 0),
-ES18XX_DOUBLE("Mono Playback Volume", 0, 0x6d, 0x6d, 4, 0, 15, 0),
 ES18XX_DOUBLE("Mic Playback Volume", 0, 0x1a, 0x1a, 4, 0, 15, 0),
 ES18XX_DOUBLE("Aux Playback Volume", 0, 0x3a, 0x3a, 4, 0, 15, 0),
-ES18XX_SINGLE("PC Speaker Playback Volume", 0, 0x3c, 0, 7, 0),
 ES18XX_SINGLE("Record Monitor", 0, 0xa8, 3, 1, 0),
 ES18XX_DOUBLE("Capture Volume", 0, 0xb4, 0xb4, 4, 0, 15, 0),
-ES18XX_SINGLE("Capture Switch", 0, 0x1c, 4, 1, 1),
 {
 	.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
 	.name = "Capture Source",
@@ -1203,19 +1206,34 @@
 }
 };
 
-static struct snd_kcontrol_new snd_es18xx_mono_in_control = 
-ES18XX_DOUBLE("Mono Input Playback Volume", 0, 0x6d, 0x6d, 4, 0, 15, 0);
-
 static struct snd_kcontrol_new snd_es18xx_recmix_controls[] = {
 ES18XX_DOUBLE("PCM Capture Volume", 0, 0x69, 0x69, 4, 0, 15, 0),
 ES18XX_DOUBLE("Mic Capture Volume", 0, 0x68, 0x68, 4, 0, 15, 0),
 ES18XX_DOUBLE("Line Capture Volume", 0, 0x6e, 0x6e, 4, 0, 15, 0),
 ES18XX_DOUBLE("FM Capture Volume", 0, 0x6b, 0x6b, 4, 0, 15, 0),
-ES18XX_DOUBLE("Mono Capture Volume", 0, 0x6f, 0x6f, 4, 0, 15, 0),
 ES18XX_DOUBLE("CD Capture Volume", 0, 0x6a, 0x6a, 4, 0, 15, 0),
 ES18XX_DOUBLE("Aux Capture Volume", 0, 0x6c, 0x6c, 4, 0, 15, 0)
 };
 
+/*
+ * These chipset specific mixer controls are initialized in
+ * snd_es18xx_mixer. Those controls which are not UNSET are then
+ * instantiated.
+ */
+static char UNSETbuf = '\0';
+#define UNSET ((char *)&UNSETbuf)
+static enum optionalControlsIndex {reg1cBit4, reg3cMono, reg6d, reg6f};
+static struct snd_kcontrol_new snd_es18xx_optional_controls[] = {
+ES18XX_SINGLE(UNSET, 0, 0x1c, 4, 1, 1),
+ES18XX_SINGLE(UNSET, 0, 0x3c, 0, 7, 0),
+ES18XX_DOUBLE(UNSET, 0, 0x6d, 0x6d, 4, 0, 15, 0),
+ES18XX_DOUBLE(UNSET, 0, 0x6f, 0x6f, 4, 0, 15, 0)
+};
+static char* captureSwitchName = "Capture Switch";
+static char* speakerPlayName = "PC Speaker Playback Volume";
+static char* monoPlayName = "Mono Playback Volume";
+static char* monoRecName = "Mono Capture Volume";
+
 static struct snd_kcontrol_new snd_es18xx_pcm1_controls[] = {
 ES18XX_DOUBLE("PCM Playback Volume", 0, 0x14, 0x14, 4, 0, 15, 0),
 };
@@ -1466,11 +1484,14 @@
 	}
 			
         outb(0x40, chip->port + 0x04);
+	udelay(10);
 	hi = inb(chip->port + 0x05);
+	udelay(10);
 	lo = inb(chip->port + 0x05);
 	if (hi != lo) {
 		chip->version = hi << 8 | lo;
 		chip->ctrl_port = inb(chip->port + 0x05) << 8;
+		udelay(10);
 		chip->ctrl_port += inb(chip->port + 0x05);
 
 		if ((chip->res_ctrl_port = request_region(chip->ctrl_port, 8, "ES18xx - CTRL")) == NULL) {
@@ -1744,7 +1765,7 @@
 static int __devinit snd_es18xx_mixer(struct snd_es18xx *chip)
 {
 	struct snd_card *card;
-	int err;
+	int err, retVal;
 	unsigned int idx;
 
 	card = chip->card;
@@ -1781,10 +1802,6 @@
 		}
 	}
 
-	if (chip->caps & ES18XX_MONO) {
-		if ((err = snd_ctl_add(card, snd_ctl_new1(&snd_es18xx_mono_in_control, chip))) < 0)
-			return err;
-	}
 	if (chip->caps & ES18XX_RECMIX) {
 		for (idx = 0; idx < ARRAY_SIZE(snd_es18xx_recmix_controls); idx++) {
 			if ((err = snd_ctl_add(card, snd_ctl_new1(&snd_es18xx_recmix_controls[idx], chip))) < 0)
@@ -1822,7 +1839,27 @@
 			
 		}
 	}
-	return 0;
+/* finish initializing other chipset specific controls
+ */
+	if (chip->version != 0x1868) {
+		snd_es18xx_optional_controls[reg3cMono].name = speakerPlayName;
+	}
+	switch (chip->version) {
+	case 0x1869:
+		snd_es18xx_optional_controls[reg1cBit4].name = captureSwitchName;
+		snd_es18xx_optional_controls[reg6d].name = monoPlayName;
+		snd_es18xx_optional_controls[reg6f].name = monoRecName;
+		break;
+	}
+	retVal = 0;
+	for (idx = 0; idx < ARRAY_SIZE(snd_es18xx_optional_controls); idx++) {
+		if (snd_es18xx_optional_controls[idx].name == UNSET)
+			continue;
+		if ((err = snd_ctl_add(card, snd_ctl_new1(&snd_es18xx_optional_controls[idx], chip))) < 0)
+			retVal = err;
+		snd_es18xx_optional_controls[idx].name = UNSET;
+	}
+	return retVal;
 }
        

Re: [es18xx.c PATCH] #1/4 for Zoom Video - resolve common vs chipset specific mixer controls

[Takashi Iwai]

At Sun, 08 Jan 2006 18:01:21 -0500,
Mark Salazar wrote:
> 
> Takashi Iwai wrote:
> 
> >This doesn't work for the multiple cards because you overwrite the
> >static array.  
> >
> Yup. Good catch. Chalk up another one for peer review.
> 
> >Better to create once kcontrol instaces via
> >snd_ctl_new1(), then copy the name on each instance according to the
> >model.
> >  
> >
> Hmmm .. what if I just reset it to UNSET in the loop where it's used (as 
> this patch does)?

Well, I think the below is more straightforward.
Please let me know if it works, then I'll merge your patches to CVS.

Thanks,

Takashi


Index: alsa-kernel/isa/es18xx.c
===================================================================
RCS file: /home/iwai/cvs/alsa/alsa-kernel/isa/es18xx.c,v
retrieving revision 1.58
diff -u -r1.58 es18xx.c
--- alsa-kernel/isa/es18xx.c	4 Jan 2006 15:10:33 -0000	1.58
+++ alsa-kernel/isa/es18xx.c	9 Jan 2006 12:13:24 -0000
@@ -1191,19 +1191,22 @@
 	return change;
 }
 
+/* Mixer controls
+ * These arrays contain setup data for mixer controls.
+ * 
+ * The controls that are universal to all chipsets are fully initialized
+ * here.
+ */
 static struct snd_kcontrol_new snd_es18xx_base_controls[] = {
 ES18XX_DOUBLE("Master Playback Volume", 0, 0x60, 0x62, 0, 0, 63, 0),
 ES18XX_DOUBLE("Master Playback Switch", 0, 0x60, 0x62, 6, 6, 1, 1),
 ES18XX_DOUBLE("Line Playback Volume", 0, 0x3e, 0x3e, 4, 0, 15, 0),
 ES18XX_DOUBLE("CD Playback Volume", 0, 0x38, 0x38, 4, 0, 15, 0),
 ES18XX_DOUBLE("FM Playback Volume", 0, 0x36, 0x36, 4, 0, 15, 0),
-ES18XX_DOUBLE("Mono Playback Volume", 0, 0x6d, 0x6d, 4, 0, 15, 0),
 ES18XX_DOUBLE("Mic Playback Volume", 0, 0x1a, 0x1a, 4, 0, 15, 0),
 ES18XX_DOUBLE("Aux Playback Volume", 0, 0x3a, 0x3a, 4, 0, 15, 0),
-ES18XX_SINGLE("PC Speaker Playback Volume", 0, 0x3c, 0, 7, 0),
 ES18XX_SINGLE("Record Monitor", 0, 0xa8, 3, 1, 0),
 ES18XX_DOUBLE("Capture Volume", 0, 0xb4, 0xb4, 4, 0, 15, 0),
-ES18XX_SINGLE("Capture Switch", 0, 0x1c, 4, 1, 1),
 {
 	.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
 	.name = "Capture Source",
@@ -1213,19 +1216,27 @@
 }
 };
 
-static struct snd_kcontrol_new snd_es18xx_mono_in_control = 
-ES18XX_DOUBLE("Mono Input Playback Volume", 0, 0x6d, 0x6d, 4, 0, 15, 0);
-
 static struct snd_kcontrol_new snd_es18xx_recmix_controls[] = {
 ES18XX_DOUBLE("PCM Capture Volume", 0, 0x69, 0x69, 4, 0, 15, 0),
 ES18XX_DOUBLE("Mic Capture Volume", 0, 0x68, 0x68, 4, 0, 15, 0),
 ES18XX_DOUBLE("Line Capture Volume", 0, 0x6e, 0x6e, 4, 0, 15, 0),
 ES18XX_DOUBLE("FM Capture Volume", 0, 0x6b, 0x6b, 4, 0, 15, 0),
-ES18XX_DOUBLE("Mono Capture Volume", 0, 0x6f, 0x6f, 4, 0, 15, 0),
 ES18XX_DOUBLE("CD Capture Volume", 0, 0x6a, 0x6a, 4, 0, 15, 0),
 ES18XX_DOUBLE("Aux Capture Volume", 0, 0x6c, 0x6c, 4, 0, 15, 0)
 };
 
+/*
+ * The chipset specific mixer controls
+ */
+static struct snd_kcontrol_new snd_es18xx_opt_speaker =
+	ES18XX_SINGLE("PC Speaker Playback Volume", 0, 0x3c, 0, 7, 0);
+
+static struct snd_kcontrol_new snd_es18xx_opt_1869[] = {
+ES18XX_SINGLE("Capture Switch", 0, 0x1c, 4, 1, 1),
+ES18XX_DOUBLE("Mono Playback Volume", 0, 0x6d, 0x6d, 4, 0, 15, 0),
+ES18XX_DOUBLE("Mono Capture Volume", 0, 0x6f, 0x6f, 4, 0, 15, 0)
+};
+
 static struct snd_kcontrol_new snd_es18xx_pcm1_controls[] = {
 ES18XX_DOUBLE("PCM Playback Volume", 0, 0x14, 0x14, 4, 0, 15, 0),
 };
@@ -1476,11 +1487,14 @@
 	}
 			
         outb(0x40, chip->port + 0x04);
+	udelay(10);
 	hi = inb(chip->port + 0x05);
+	udelay(10);
 	lo = inb(chip->port + 0x05);
 	if (hi != lo) {
 		chip->version = hi << 8 | lo;
 		chip->ctrl_port = inb(chip->port + 0x05) << 8;
+		udelay(10);
 		chip->ctrl_port += inb(chip->port + 0x05);
 
 		if ((chip->res_ctrl_port = request_region(chip->ctrl_port, 8, "ES18xx - CTRL")) == NULL) {
@@ -1778,10 +1792,6 @@
 		}
 	}
 
-	if (chip->caps & ES18XX_MONO) {
-		if ((err = snd_ctl_add(card, snd_ctl_new1(&snd_es18xx_mono_in_control, chip))) < 0)
-			return err;
-	}
 	if (chip->caps & ES18XX_RECMIX) {
 		for (idx = 0; idx < ARRAY_SIZE(snd_es18xx_recmix_controls); idx++) {
 			if ((err = snd_ctl_add(card, snd_ctl_new1(&snd_es18xx_recmix_controls[idx], chip))) < 0)
@@ -1819,6 +1829,23 @@
 			
 		}
 	}
+	/* finish initializing other chipset specific controls
+	 */
+	if (chip->version != 0x1868) {
+		err = snd_ctl_add(card, snd_ctl_new1(&snd_es18xx_opt_speaker,
+						     chip));
+		if (err < 0)
+			return err;
+	}
+	if (chip->version == 0x1869) {
+		for (idx = 0; idx < ARRAY_SIZE(snd_es18xx_opt_1869); idx++) {
+			err = snd_ctl_add(card,
+					  snd_ctl_new1(&snd_es18xx_opt_1869[idx],
+						       chip));
+			if (err < 0)
+				return err;
+		}
+	}
 	return 0;
 }
        


-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click

Re: [es18xx.c PATCH] #1/4 for Zoom Video - resolve common vs chipset specific mixer controls

[Mark Salazar]

Takashi Iwai wrote:

>
>Well, I think the below is more straightforward.
>Please let me know if it works, then I'll merge your patches to CVS.
>  
>
Haven't tried yet, but it looks like it should work. If we go this way 
it'll break my 4th patch though where I add the controls for Zoom Video 
so I'll have to rework that as well. I expect the modified version would 
look something like:
- adding a "Video Playback Switch" switch control to snd_es18xx_opt_1869;
- adding a "snd_es18xx_opt_1878" to hold a single volume control "Video 
Playback Volume";
- adding a "snd_es18xx_opt_1879" to hold stereo volume "Video Playback 
Volume",  "Video Playback Volume", and the "Video Playback Switch" controls.
along with the necessary instantiation logic in snd_es18xx_mixer. The 
reason I used one array to hold all the chipset specific control 
information was because I saw the same registers used for different 
controls in different chipsets and I was trying to keep things simple 
and readable (and obvious swtich and single instantiation loop) in 
snd_es18xx_mixer. However this is my first time working ALSA code, and I 
will defer to your opinion.
Which way do you want to go?



-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click

Re: [es18xx.c PATCH] #1/4 for Zoom Video - resolve common vs chipset specific mixer controls

[Mark Salazar]

Takashi Iwai wrote:

>
>Basically, rewriting the static array and restoring it looks odd, so
>I prefer having multiple kcontrol_new arrays.  (Besides, you could use
>simply NULL for the validity check.)  From the readability viewpoint,
>separate arrays are better, too.  The most important point is that the
>code flow is straightforward. 
>
>
>Takashi
>
>  
>
OK, I've tried your version of mod1 and it seems to work fine.

Also retested the patches for mods 2 & 3 ... no changes necessary.

Revised mod4 patch to follow.


-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click

Re: [es18xx.c PATCH] #1/4 for Zoom Video - resolve common vs chipset specific mixer controls

[Takashi Iwai]

At Sun, 15 Jan 2006 15:30:06 -0500,
Mark Salazar wrote:
> 
> Takashi Iwai wrote:
> 
> >
> >Basically, rewriting the static array and restoring it looks odd, so
> >I prefer having multiple kcontrol_new arrays.  (Besides, you could use
> >simply NULL for the validity check.)  From the readability viewpoint,
> >separate arrays are better, too.  The most important point is that the
> >code flow is straightforward. 
> >
> >
> >Takashi
> >
> >  
> >
> OK, I've tried your version of mod1 and it seems to work fine.
> 
> Also retested the patches for mods 2 & 3 ... no changes necessary.
> 
> Revised mod4 patch to follow.

OK, I applied all 4 patches now.  Thanks!


Takashi


-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click