[PATCH] ess1688: fix OPL3 port setting

[PATCH] ess1688: fix OPL3 port setting

[Krzysztof Helt]

From: Krzysztof Helt <krzysztof.h1@wp.pl>

The ess1688 driver uses the same port
for PCM audio (SB compatible) and OPL3
synthesis. It is wrong so allow to
choose a correct port for OPL3 synthesis.

Signed-off-by: Krzysztof Helt <krzysztof.h1@wp.pl>
---
The bug  was entered as issue #282 and closed due
to lack of response. However, the code is plainly wrong
as MS Win95 driver reserves two separate IO areas 
starting from 0x220 and 0x388 for ess688 and the
third 0x300 for ess1688.

I have no card to test as I cut my es1688 to reuse 
its audio jacks few months ago.

 sound/isa/es1688/es1688.c |   20 +++++++++++++-------
 1 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/sound/isa/es1688/es1688.c b/sound/isa/es1688/es1688.c
index b463771..5e4d766 100644
--- a/sound/isa/es1688/es1688.c
+++ b/sound/isa/es1688/es1688.c
@@ -49,6 +49,7 @@ static int index[SNDRV_CARDS] = SNDRV_DEFAULT_IDX;	/* Index 0-MAX */
 static char *id[SNDRV_CARDS] = SNDRV_DEFAULT_STR;	/* ID for this card */
 static int enable[SNDRV_CARDS] = SNDRV_DEFAULT_ENABLE;	/* Enable this card */
 static long port[SNDRV_CARDS] = SNDRV_DEFAULT_PORT;	/* 0x220,0x240,0x260 */
+static long fm_port[SNDRV_CARDS] = SNDRV_DEFAULT_PORT;	/* Usually 0x388 */
 static long mpu_port[SNDRV_CARDS] = {[0 ... (SNDRV_CARDS - 1)] = -1};
 static int irq[SNDRV_CARDS] = SNDRV_DEFAULT_IRQ;	/* 5,7,9,10 */
 static int mpu_irq[SNDRV_CARDS] = SNDRV_DEFAULT_IRQ;	/* 5,7,9,10 */
@@ -65,6 +66,8 @@ MODULE_PARM_DESC(port, "Port # for " CRD_NAME " driver.");
 module_param_array(mpu_port, long, NULL, 0444);
 MODULE_PARM_DESC(mpu_port, "MPU-401 port # for " CRD_NAME " driver.");
 module_param_array(irq, int, NULL, 0444);
+module_param_array(fm_port, long, NULL, 0444);
+MODULE_PARM_DESC(fm_port, "FM port # for ES1688 driver.");
 MODULE_PARM_DESC(irq, "IRQ # for " CRD_NAME " driver.");
 module_param_array(mpu_irq, int, NULL, 0444);
 MODULE_PARM_DESC(mpu_irq, "MPU-401 IRQ # for " CRD_NAME " driver.");
@@ -143,13 +146,16 @@ static int __devinit snd_es1688_probe(struct device *dev, unsigned int n)
 	sprintf(card->longname, "%s at 0x%lx, irq %i, dma %i", pcm->name,
 		chip->port, chip->irq, chip->dma8);
 
-	if (snd_opl3_create(card, chip->port, chip->port + 2,
-			OPL3_HW_OPL3, 0, &opl3) < 0)
-		dev_warn(dev, "opl3 not detected at 0x%lx\n", chip->port);
-	else {
-		error =	snd_opl3_hwdep_new(opl3, 0, 1, NULL);
-		if (error < 0)
-			goto out;
+	if (fm_port[n] > 0 && fm_port[n] != SNDRV_AUTO_PORT) {
+		if (snd_opl3_create(card, fm_port[n], fm_port[n] + 2,
+				OPL3_HW_OPL3, 0, &opl3) < 0)
+			dev_warn(dev,
+				 "opl3 not detected at 0x%lx\n", fm_port[n]);
+		else {
+			error =	snd_opl3_hwdep_new(opl3, 0, 1, NULL);
+			if (error < 0)
+				goto out;
+		}
 	}
 
 	if (mpu_irq[n] >= 0 && mpu_irq[n] != SNDRV_AUTO_IRQ &&
-- 
1.5.2.2


-----------------------------------------------------------------------
Promocja w Speak Up. Kwartal angielskiego za darmo. 
3 miesiace nauki gratis. Sprawdz teraz! >> http://link.interia.pl/f2019

Re: [PATCH] ess1688: fix OPL3 port setting

[Rene Herman]

On 25-01-09 21:10, Krzysztof Helt wrote:

> From: Krzysztof Helt <krzysztof.h1@wp.pl>
> 
> The ess1688 driver uses the same port
> for PCM audio (SB compatible) and OPL3
> synthesis. It is wrong so allow to
> choose a correct port for OPL3 synthesis.

Only a very quick reply -- generally, the low 2 SB ports are indeed OPL3 
compatible (or was it just OPL2? ...)

Rene.

Re: [PATCH] ess1688: fix OPL3 port setting

[Takashi Iwai]

At Sun, 25 Jan 2009 23:05:55 +0100,
Rene Herman wrote:
> 
> On 25-01-09 21:10, Krzysztof Helt wrote:
> 
> > From: Krzysztof Helt <krzysztof.h1@wp.pl>
> > 
> > The ess1688 driver uses the same port
> > for PCM audio (SB compatible) and OPL3
> > synthesis. It is wrong so allow to
> > choose a correct port for OPL3 synthesis.
> 
> Only a very quick reply -- generally, the low 2 SB ports are indeed OPL3 
> compatible (or was it just OPL2? ...)

All devices have separate OPL3 I/O ports.  So there must be a
reason...


Takashi

Re: [PATCH] ess1688: fix OPL3 port setting

[Krzysztof Helt]

On Mon, 26 Jan 2009 10:42:29 +0100
Takashi Iwai <tiwai@suse.de> wrote:

> At Sun, 25 Jan 2009 23:05:55 +0100,
> Rene Herman wrote:
> > 
> > On 25-01-09 21:10, Krzysztof Helt wrote:
> > 
> > > From: Krzysztof Helt <krzysztof.h1@wp.pl>
> > > 
> > > The ess1688 driver uses the same port
> > > for PCM audio (SB compatible) and OPL3
> > > synthesis. It is wrong so allow to
> > > choose a correct port for OPL3 synthesis.
> > 
> > Only a very quick reply -- generally, the low 2 SB ports are indeed OPL3 
> > compatible (or was it just OPL2? ...)
> 
> All devices have separate OPL3 I/O ports.  So there must be a
> reason...
> 

The ESS 688 datasheet states that FM chip select signal (FMCSB) 
is activated when one of the port ranges: 
388H-389H, 2x8H-2x9H, 2x0H-2x3H
is selected. The current code may work on on the ESS 688. 
However, it obviously does not work with the ESS 1688 (bug #282).
The bug report states that the OPL3 synthesis works with the
OSS driver which chooses the 0x388 port for the OPL3.

My patch adds a new port setting so it is still possible to set
the fm_port value the same as the port value. No change here.
However, it allows setting different values to the port and the
fm_port setting if needed.

Takashi, please consider applying my patch to the alsa tree.

Regards,
Krzysztof

----------------------------------------------------------------------
Speak Up. Angielski szybko i skutecznie. 3 miesiace nauki gratis.
Sprawdz. >> http://link.interia.pl/f2019

Re: [PATCH] ess1688: fix OPL3 port setting

[Takashi Iwai]

At Thu, 29 Jan 2009 15:55:27 +0100,
Krzysztof Helt wrote:
> 
> On Mon, 26 Jan 2009 10:42:29 +0100
> Takashi Iwai <tiwai@suse.de> wrote:
> 
> > At Sun, 25 Jan 2009 23:05:55 +0100,
> > Rene Herman wrote:
> > > 
> > > On 25-01-09 21:10, Krzysztof Helt wrote:
> > > 
> > > > From: Krzysztof Helt <krzysztof.h1@wp.pl>
> > > > 
> > > > The ess1688 driver uses the same port
> > > > for PCM audio (SB compatible) and OPL3
> > > > synthesis. It is wrong so allow to
> > > > choose a correct port for OPL3 synthesis.
> > > 
> > > Only a very quick reply -- generally, the low 2 SB ports are indeed OPL3 
> > > compatible (or was it just OPL2? ...)
> > 
> > All devices have separate OPL3 I/O ports.  So there must be a
> > reason...
> > 
> 
> The ESS 688 datasheet states that FM chip select signal (FMCSB) 
> is activated when one of the port ranges: 
> 388H-389H, 2x8H-2x9H, 2x0H-2x3H
> is selected. The current code may work on on the ESS 688. 
> However, it obviously does not work with the ESS 1688 (bug #282).
> The bug report states that the OPL3 synthesis works with the
> OSS driver which chooses the 0x388 port for the OPL3.
> 
> My patch adds a new port setting so it is still possible to set
> the fm_port value the same as the port value. No change here.
> However, it allows setting different values to the port and the
> fm_port setting if needed.

Fair enough.  However...

> Takashi, please consider applying my patch to the alsa tree.

One thing I noticed in your patch is that opl3 won't be created after
your change as default, as fmport=SNDRV_AUTO_PORT.  This is a
regression.  The code should be like below instead:

	if (fm_port[n] == SNDRV_AUTO_PORT)
		fm_port[n] = chip->port; /* share the same port */

	if (fm_port[n] > 0) {
		if (snd_opl3_create(card, fm_port[n], fm_port[n] + 2,
                              OPL3_HW_OPL3, 0, &opl3) < 0)
			      ...

Could you fix and repost?


thanks,

Takashi

[PATCH] ess1688: fix OPL3 port setting (2nd rev)

[Krzysztof Helt]

From: Krzysztof Helt <krzysztof.h1@wp.pl>

The ess1688 driver uses the same port
for PCM audio (SB compatible) and OPL3
synthesis. It is not always right so allow to
choose a different port for OPL3 synthesis.

Signed-off-by: Krzysztof Helt <krzysztof.h1@wp.pl>
---
Changed fm_port behaviour after Takashi's request.

diff --git a/sound/isa/es1688/es1688.c b/sound/isa/es1688/es1688.c
index b463771..e7fbb06 100644
--- a/sound/isa/es1688/es1688.c
+++ b/sound/isa/es1688/es1688.c
@@ -49,6 +49,7 @@ static int index[SNDRV_CARDS] = SNDRV_DEFAULT_IDX;	/* Index 0-MAX */
 static char *id[SNDRV_CARDS] = SNDRV_DEFAULT_STR;	/* ID for this card */
 static int enable[SNDRV_CARDS] = SNDRV_DEFAULT_ENABLE;	/* Enable this card */
 static long port[SNDRV_CARDS] = SNDRV_DEFAULT_PORT;	/* 0x220,0x240,0x260 */
+static long fm_port[SNDRV_CARDS] = SNDRV_DEFAULT_PORT;	/* Usually 0x388 */
 static long mpu_port[SNDRV_CARDS] = {[0 ... (SNDRV_CARDS - 1)] = -1};
 static int irq[SNDRV_CARDS] = SNDRV_DEFAULT_IRQ;	/* 5,7,9,10 */
 static int mpu_irq[SNDRV_CARDS] = SNDRV_DEFAULT_IRQ;	/* 5,7,9,10 */
@@ -65,6 +66,8 @@ MODULE_PARM_DESC(port, "Port # for " CRD_NAME " driver.");
 module_param_array(mpu_port, long, NULL, 0444);
 MODULE_PARM_DESC(mpu_port, "MPU-401 port # for " CRD_NAME " driver.");
 module_param_array(irq, int, NULL, 0444);
+module_param_array(fm_port, long, NULL, 0444);
+MODULE_PARM_DESC(fm_port, "FM port # for ES1688 driver.");
 MODULE_PARM_DESC(irq, "IRQ # for " CRD_NAME " driver.");
 module_param_array(mpu_irq, int, NULL, 0444);
 MODULE_PARM_DESC(mpu_irq, "MPU-401 IRQ # for " CRD_NAME " driver.");
@@ -143,13 +146,19 @@ static int __devinit snd_es1688_probe(struct device *dev, unsigned int n)
 	sprintf(card->longname, "%s at 0x%lx, irq %i, dma %i", pcm->name,
 		chip->port, chip->irq, chip->dma8);
 
-	if (snd_opl3_create(card, chip->port, chip->port + 2,
-			OPL3_HW_OPL3, 0, &opl3) < 0)
-		dev_warn(dev, "opl3 not detected at 0x%lx\n", chip->port);
-	else {
-		error =	snd_opl3_hwdep_new(opl3, 0, 1, NULL);
-		if (error < 0)
-			goto out;
+	if (fm_port[n] != SNDRV_AUTO_PORT)
+		fm_port[n] = port[n];	/* share the same port */
+
+	if (fm_port[n] > 0) {
+		if (snd_opl3_create(card, fm_port[n], fm_port[n] + 2,
+				OPL3_HW_OPL3, 0, &opl3) < 0)
+			dev_warn(dev,
+				 "opl3 not detected at 0x%lx\n", fm_port[n]);
+		else {
+			error =	snd_opl3_hwdep_new(opl3, 0, 1, NULL);
+			if (error < 0)
+				goto out;
+		}
 	}
 
 	if (mpu_irq[n] >= 0 && mpu_irq[n] != SNDRV_AUTO_IRQ &&

-----------------------------------------------------------------------
Promocja w Speak Up. Kwartal angielskiego za darmo. 
3 miesiace nauki gratis. Sprawdz teraz! >> http://link.interia.pl/f2019

Re: [PATCH] ess1688: fix OPL3 port setting (2nd rev)

[Takashi Iwai]

At Fri, 30 Jan 2009 09:55:28 +0100,
Krzysztof Helt wrote:
> 
> @@ -143,13 +146,19 @@ static int __devinit snd_es1688_probe(struct device *dev, unsigned int n)
>  	sprintf(card->longname, "%s at 0x%lx, irq %i, dma %i", pcm->name,
>  		chip->port, chip->irq, chip->dma8);
>  
> -	if (snd_opl3_create(card, chip->port, chip->port + 2,
> -			OPL3_HW_OPL3, 0, &opl3) < 0)
> -		dev_warn(dev, "opl3 not detected at 0x%lx\n", chip->port);
> -	else {
> -		error =	snd_opl3_hwdep_new(opl3, 0, 1, NULL);
> -		if (error < 0)
> -			goto out;
> +	if (fm_port[n] != SNDRV_AUTO_PORT)
> +		fm_port[n] = port[n];	/* share the same port */

It should be

	if (fm_port[n] == SNDRV_AUTO_PORT)


Takashi

[PATCH] ess1688: fix OPL3 port setting (3rd rev)

[Krzysztof Helt]

From: Krzysztof Helt <krzysztof.h1@wp.pl>

The ess1688 driver uses the same port
for PCM audio (SB compatible) and OPL3
synthesis. It is not always right so allow to
choose a different port for OPL3 synthesis.

Signed-off-by: Krzysztof Helt <krzysztof.h1@wp.pl>
---
I am sorry for stupid copy-paste error. It is fixed now.

diff --git a/sound/isa/es1688/es1688.c b/sound/isa/es1688/es1688.c
index b463771..e7fbb06 100644
--- a/sound/isa/es1688/es1688.c
+++ b/sound/isa/es1688/es1688.c
@@ -49,6 +49,7 @@ static int index[SNDRV_CARDS] = SNDRV_DEFAULT_IDX;	/* Index 0-MAX */
 static char *id[SNDRV_CARDS] = SNDRV_DEFAULT_STR;	/* ID for this card */
 static int enable[SNDRV_CARDS] = SNDRV_DEFAULT_ENABLE;	/* Enable this card */
 static long port[SNDRV_CARDS] = SNDRV_DEFAULT_PORT;	/* 0x220,0x240,0x260 */
+static long fm_port[SNDRV_CARDS] = SNDRV_DEFAULT_PORT;	/* Usually 0x388 */
 static long mpu_port[SNDRV_CARDS] = {[0 ... (SNDRV_CARDS - 1)] = -1};
 static int irq[SNDRV_CARDS] = SNDRV_DEFAULT_IRQ;	/* 5,7,9,10 */
 static int mpu_irq[SNDRV_CARDS] = SNDRV_DEFAULT_IRQ;	/* 5,7,9,10 */
@@ -65,6 +66,8 @@ MODULE_PARM_DESC(port, "Port # for " CRD_NAME " driver.");
 module_param_array(mpu_port, long, NULL, 0444);
 MODULE_PARM_DESC(mpu_port, "MPU-401 port # for " CRD_NAME " driver.");
 module_param_array(irq, int, NULL, 0444);
+module_param_array(fm_port, long, NULL, 0444);
+MODULE_PARM_DESC(fm_port, "FM port # for ES1688 driver.");
 MODULE_PARM_DESC(irq, "IRQ # for " CRD_NAME " driver.");
 module_param_array(mpu_irq, int, NULL, 0444);
 MODULE_PARM_DESC(mpu_irq, "MPU-401 IRQ # for " CRD_NAME " driver.");
@@ -143,13 +146,19 @@ static int __devinit snd_es1688_probe(struct device *dev, unsigned int n)
 	sprintf(card->longname, "%s at 0x%lx, irq %i, dma %i", pcm->name,
 		chip->port, chip->irq, chip->dma8);
 
-	if (snd_opl3_create(card, chip->port, chip->port + 2,
-			OPL3_HW_OPL3, 0, &opl3) < 0)
-		dev_warn(dev, "opl3 not detected at 0x%lx\n", chip->port);
-	else {
-		error =	snd_opl3_hwdep_new(opl3, 0, 1, NULL);
-		if (error < 0)
-			goto out;
+	if (fm_port[n] == SNDRV_AUTO_PORT)
+		fm_port[n] = port[n];	/* share the same port */
+
+	if (fm_port[n] > 0) {
+		if (snd_opl3_create(card, fm_port[n], fm_port[n] + 2,
+				OPL3_HW_OPL3, 0, &opl3) < 0)
+			dev_warn(dev,
+				 "opl3 not detected at 0x%lx\n", fm_port[n]);
+		else {
+			error =	snd_opl3_hwdep_new(opl3, 0, 1, NULL);
+			if (error < 0)
+				goto out;
+		}
 	}
 
 	if (mpu_irq[n] >= 0 && mpu_irq[n] != SNDRV_AUTO_IRQ &&

-----------------------------------------------------------------------
Promocja w Speak Up. Kwartal angielskiego za darmo. 
3 miesiace nauki gratis. Sprawdz teraz! >> http://link.interia.pl/f2019

Re: [PATCH] ess1688: fix OPL3 port setting (3rd rev)

[Takashi Iwai]

At Fri, 30 Jan 2009 19:20:29 +0100,
Krzysztof Helt wrote:
> 
> From: Krzysztof Helt <krzysztof.h1@wp.pl>
> 
> The ess1688 driver uses the same port
> for PCM audio (SB compatible) and OPL3
> synthesis. It is not always right so allow to
> choose a different port for OPL3 synthesis.
> 
> Signed-off-by: Krzysztof Helt <krzysztof.h1@wp.pl>

Applied now.  Thanks!


Takashi