Skip to contentSkip to conversation

ALSA in 2.6 failing to find the OPL chip of the sb cards

DISCUSSION Santiago Garcia Mantinan 4 participants 17 messages Jan 7, 2004 – Jan 18, 2004

Subsystems (3)
Sound Pnp support Isapnp
lore
Share & tools
lore

Santiago Garcia Mantinan

author
#0 lore raw

Hi!

I have a SB16PNP on which alsa under kernel 2.6 fails to detect the OPL chip, I have tried the 0.9.7 version wich comes with the kernel (up to version 2.6.1-rc2) and now even alsa 1.0.0rc2 compiled for the 2.6.1-rc2 kernel, on both I get the same result:

sb16: no OPL device at 0x388-0x38a

this is the full output on version 1.0.0rc2:

Starting ALSA (version 1.0.0rc2):pnp: Device 00:01.00 activated. ALSA /usr/src/modules/alsa-driver/alsa-kernel/isa/sb/sb16.c:489: sb16: no OPL device at 0x388-0x38a
sb16.

Alsa version 0.9.8 works perfectly under 2.4.X.

Don't hesitate to contact me for any other info that may be needed to track this.

Regards...

Signature · 1 line
Manty/BestiaTester -> http://manty.net
#1 lore raw

At Wed, 7 Jan 2004 22:29:16 +0100,

Quoted 8 lines · Santiago Garcia Mantinan

Hi!

I have a SB16PNP on which alsa under kernel 2.6 fails to detect the OPL chip, I have tried the 0.9.7 version wich comes with the kernel (up to version 2.6.1-rc2) and now even alsa 1.0.0rc2 compiled for the 2.6.1-rc2 kernel, on both I get the same result:

Quoted 8 lines

sb16: no OPL device at 0x388-0x38a

this is the full output on version 1.0.0rc2:

Starting ALSA (version 1.0.0rc2):pnp: Device 00:01.00 activated. ALSA /usr/src/modules/alsa-driver/alsa-kernel/isa/sb/sb16.c:489: sb16: no OPL device at 0x388-0x38a
sb16.

compile with CONFIG_SND_DEBUG and CONFIG_SND_VERBOSE_PRINTK. if it's in snd_opl3_detect(), "OPL2/3 chip not detected at ..." message should appear (together with other message in snd_opl3_detect()).
if not, it must be in snd_device_new(), and quite mysterious...

--
Takashi Iwai <[email protected]> ALSA Developer - www.alsa-project.org

author
#2 lore raw
Quoted 5 lines

compile with CONFIG_SND_DEBUG and CONFIG_SND_VERBOSE_PRINTK. if it's in snd_opl3_detect(), "OPL2/3 chip not detected at ..." message should appear (together with other message in snd_opl3_detect()).
if not, it must be in snd_device_new(), and quite mysterious...

I have compiled 2.6.1rc3 this way, here is the result when loading the kernel's alsa modules for sb16 (0.9.7):

pnp: Device 00:01.00 activated.
ALSA sound/isa/sb/sb16.c:313: pnp SB16: port=0x220, mpu port=0x330, fm port=0x388 ALSA sound/isa/sb/sb16.c:315: pnp SB16: dma1=1, dma2=5, irq=10
ALSA sound/isa/sb/sb_common.c:133: SB [0x220]: DSP chip found, version = 4.13 ALSA sound/drivers/opl3/opl3_lib.c:133: OPL3: stat1 = 0xff
ALSA sound/drivers/opl3/opl3_lib.c:444: OPL2/3 chip not detected at 0x388/0x38a ALSA sound/isa/sb/sb16.c:484: sb16: no OPL device at 0x388-0x38a

Hope this helps a bit.

Don't hesitate to ask for anything else you may need.

Regards...

Signature · 1 line
Manty/BestiaTester -> http://manty.net
author
#3 lore raw
In reply to Takashi Iwai
Quoted 4 lines

compile with CONFIG_SND_DEBUG and CONFIG_SND_VERBOSE_PRINTK. if it's in snd_opl3_detect(), "OPL2/3 chip not detected at ..." message should appear (together with other message in snd_opl3_detect()).

I have tested now with 2.6.1 with vanilla driver (0.9.7) and also with 1.0.1 using the patch that Jaroslav posted yesterday available at ftp://ftp.alsa-project.org/pub/kernel-patches/alsa-bk-2004-01-08.patch.gz and got the same result as before, these are the messages for 1.0.1:

pnp: Device 00:01.00 activated.
ALSA sound/isa/sb/sb16.c:313: pnp SB16: port=0x220, mpu port=0x330, fm port=0x388 ALSA sound/isa/sb/sb16.c:315: pnp SB16: dma1=1, dma2=5, irq=10
ALSA sound/isa/sb/sb_common.c:133: SB [0x220]: DSP chip found, version = 4.13 ALSA sound/drivers/opl3/opl3_lib.c:133: OPL3: stat1 = 0xff
ALSA sound/drivers/opl3/opl3_lib.c:444: OPL2/3 chip not detected at 0x388/0x38a ALSA sound/isa/sb/sb16.c:489: sb16: no OPL device at 0x388-0x38a

Hope it helps.

Regards...

Signature · 1 line
Manty/BestiaTester -> http://manty.net
#4 lore raw

At Fri, 9 Jan 2004 18:17:15 +0100,

Quoted 11 lines · Santiago Garcia Mantinan
Quoted 5 lines

compile with CONFIG_SND_DEBUG and CONFIG_SND_VERBOSE_PRINTK. if it's in snd_opl3_detect(), "OPL2/3 chip not detected at ..." message should appear (together with other message in snd_opl3_detect()).

I have tested now with 2.6.1 with vanilla driver (0.9.7) and also with 1.0.1 using the patch that Jaroslav posted yesterday available at ftp://ftp.alsa-project.org/pub/kernel-patches/alsa-bk-2004-01-08.patch.gz and got the same result as before, these are the messages for 1.0.1:

Quoted 7 lines

pnp: Device 00:01.00 activated.
ALSA sound/isa/sb/sb16.c:313: pnp SB16: port=0x220, mpu port=0x330, fm port=0x388 ALSA sound/isa/sb/sb16.c:315: pnp SB16: dma1=1, dma2=5, irq=10
ALSA sound/isa/sb/sb_common.c:133: SB [0x220]: DSP chip found, version = 4.13 ALSA sound/drivers/opl3/opl3_lib.c:133: OPL3: stat1 = 0xff
ALSA sound/drivers/opl3/opl3_lib.c:444: OPL2/3 chip not detected at 0x388/0x38a ALSA sound/isa/sb/sb16.c:489: sb16: no OPL device at 0x388-0x38a

then it fails in the reset sequence of opl chip, namely, what happens if you replace the line 441

	opl3->command = &snd_opl2_command;
with
	opl3->command = &snd_opl3_command;

?

--
Takashi Iwai <[email protected]> ALSA Developer - www.alsa-project.org

author
#5 lore raw
Quoted 5 lines

then it fails in the reset sequence of opl chip, namely, what happens if you replace the line 441

	opl3->command = &snd_opl2_command;
with
	opl3->command = &snd_opl3_command;

Looks exactly the same thing to me:

pnp: Device 00:01.00 activated.
ALSA sound/isa/sb/sb16.c:313: pnp SB16: port=0x220, mpu port=0x330, fm port=0x388 ALSA sound/isa/sb/sb16.c:315: pnp SB16: dma1=1, dma2=5, irq=10
ALSA sound/isa/sb/sb_common.c:133: SB [0x220]: DSP chip found, version = 4.13 ALSA sound/drivers/opl3/opl3_lib.c:133: OPL3: stat1 = 0xff
ALSA sound/drivers/opl3/opl3_lib.c:444: OPL2/3 chip not detected at 0x388/0x38a ALSA sound/isa/sb/sb16.c:484: sb16: no OPL device at 0x388-0x38a

I think I have already said that in 2.4 it works, and I have tested both alsa in the kernel plus alsa sources downloaded from alsa-project, this last one works in 2.4 but doesn't work in 2.6.

Regards...

Signature · 1 line
Manty/BestiaTester -> http://manty.net
#6 lore raw
Quoted 16 lines · Santiago Garcia Mantinan
Quoted 4 lines

then it fails in the reset sequence of opl chip, namely, what happens if you replace the line 441

	opl3->command = &snd_opl2_command;
with
	opl3->command = &snd_opl3_command;

Looks exactly the same thing to me:

pnp: Device 00:01.00 activated.
ALSA sound/isa/sb/sb16.c:313: pnp SB16: port=0x220, mpu port=0x330, fm port=0x388 ALSA sound/isa/sb/sb16.c:315: pnp SB16: dma1=1, dma2=5, irq=10
ALSA sound/isa/sb/sb_common.c:133: SB [0x220]: DSP chip found, version = 4.13 ALSA sound/drivers/opl3/opl3_lib.c:133: OPL3: stat1 = 0xff
ALSA sound/drivers/opl3/opl3_lib.c:444: OPL2/3 chip not detected at 0x388/0x38a ALSA sound/isa/sb/sb16.c:484: sb16: no OPL device at 0x388-0x38a

Quoted 3 lines

I think I have already said that in 2.4 it works, and I have tested both alsa in the kernel plus alsa sources downloaded from alsa-project, this last one works in 2.4 but doesn't work in 2.6.

I'm seeing the same behaviour with a Sound Blaster AWE64 Gold. It seems it's not an ALSA problem though, but ISA-PnP. Enabling the card from userspace with the old isapnp tool makes the OPL chip appear:

root@7ixe4:~# dmesg -n8
root@7ixe4:~# modprobe snd-sbawe
pnp: the driver 'sb16' has been registered
pnp: match found with the PnP device '01:01.00' and the driver 'sb16'
pnp: match found with the PnP device '01:01.02' and the driver 'sb16'
pnp: Device 01:01.00 activated.
pnp: Device 01:01.02 activated.
ALSA sound/isa/sb/sb16.c:484: sbawe: no OPL device at 0x388-0x38a
root@7ixe4:~# modprobe -r snd-sbawe
pnp: Device 01:01.00 disabled.
pnp: Device 01:01.02 disabled.
pnp: the driver 'sb16' has been unregistered
root@7ixe4:~# isapnp awe64g
Board 1 has Identity 43 0f f1 94 5c 9e 00 8c 0e:  CTL009e Serial No 267490396 [checksum 43]
Board 2 has Identity 2c 00 0f e4 18 02 00 94 16:  ETT0002 Serial No 1041432 [checksum 2c]
root@7ixe4:~# modprobe snd-sbawe
pnp: the driver 'sb16' has been registered
pnp: match found with the PnP device '01:01.00' and the driver 'sb16'
pnp: match found with the PnP device '01:01.02' and the driver 'sb16'
pnp: Device 01:01.00 activated.
pnp: Device 01:01.02 activated.
root@7ixe4:~#

("awe64g" is just "pnpdump >awe64g", with preffered config uncommented)

That is, the driver doesn't complain anymore. Still didn't see the OPL appear in /proc/ioports though (loading with fm_port=0x388 doesn't change that). Haven't investigated further yet, will do.

Rene.

#7 lore raw

Rene Herman wrote:

NOTE: I seem unable to contact Adam Belay; his ISP is not accepting mail from mine. Takashi, if you agree attached patch is a correct fix, could you relay it to Adam?

Quoted 8 lines
Quoted 6 lines

ALSA sound/drivers/opl3/opl3_lib.c:444: OPL2/3 chip not detected at 0x388/0x38a ALSA sound/isa/sb/sb16.c:484: sb16: no OPL device at 0x388-0x38a

I think I have already said that in 2.4 it works, and I have tested both alsa in the kernel plus alsa sources downloaded from alsa-project, this last one works in 2.4 but doesn't work in 2.6.

I'm seeing the same behaviour with a Sound Blaster AWE64 Gold. It seems it's not an ALSA problem though, but ISA-PnP.

Assuming ALSA isn't misusing the PnP API, it's indeed not ALSA, but PnP.

It also isn't actually an OPL3 issue, but MPU401. Trouble is that sb16.c doesn't set mpu_port to SNDRV_DEFAULT_PORT, but hardcodes the values for the first two cards as 0x330 and 0x300 (Takashi: why is that, by the way? At least for ISA-PnP cards SNDRV_DEFAULT_PORT would seem better?). This causes the initialisation code to call pnp_resource_change() for the MPU port resource, which clears IORESOURCE_AUTO for that resource.

The rest of the PnP layer, seeing IORESOURCE_AUTO clear, then never touches that resource again, but at that point IORESOURCE_IO (indicating an I/O port resource) hasn't yet been set, so that when it later gets to isapnp_set_resources(), that function bails out believing it has reached the end of the I/O port resources for the device. Since SB is resource 0, MPU401 resource 1 and OPL3 resource 2 for the device, only SB gets enabled, MPU401 and OPL3 do not.

Making sure IORESOURCE_IO gets set fixes it. Having pnp_init_resource_table() do this seems proper.

The attached patch works for me:

rene@7ixe4:~$ sbiload -l
 Port     Client name                       Port name
 64:0     Rawmidi 0 - MPU-401 (UART) 0-0    MPU-401 (UART) 0-0
 65:0     Emu8000 WaveTable                 Emu8000 Port 0
 65:1     Emu8000 WaveTable                 Emu8000 Port 1
 65:2     Emu8000 WaveTable                 Emu8000 Port 2
 65:3     Emu8000 WaveTable                 Emu8000 Port 3
 66:0     OPL3 FM synth                     OPL3 FM Port

Rene.

drivers/pnp/manager.c renamed from drivers/pnp/manager.c.orig +4 -4
@@ -223,25 +223,25 @@
 		table->irq_resource[idx].name = NULL;
 		table->irq_resource[idx].start = -1;
 		table->irq_resource[idx].end = -1;
-		table->irq_resource[idx].flags = IORESOURCE_AUTO | IORESOURCE_UNSET;
+		table->irq_resource[idx].flags = IORESOURCE_AUTO | IORESOURCE_UNSET | IORESOURCE_IRQ;
 	}
 	for (idx = 0; idx < PNP_MAX_DMA; idx++) {
 		table->dma_resource[idx].name = NULL;
 		table->dma_resource[idx].start = -1;
 		table->dma_resource[idx].end = -1;
-		table->dma_resource[idx].flags = IORESOURCE_AUTO | IORESOURCE_UNSET;
+		table->dma_resource[idx].flags = IORESOURCE_AUTO | IORESOURCE_UNSET | IORESOURCE_DMA;
 	}
 	for (idx = 0; idx < PNP_MAX_PORT; idx++) {
 		table->port_resource[idx].name = NULL;
 		table->port_resource[idx].start = 0;
 		table->port_resource[idx].end = 0;
-		table->port_resource[idx].flags = IORESOURCE_AUTO | IORESOURCE_UNSET;
+		table->port_resource[idx].flags = IORESOURCE_AUTO | IORESOURCE_UNSET | IORESOURCE_IO;
 	}
 	for (idx = 0; idx < PNP_MAX_MEM; idx++) {
 		table->mem_resource[idx].name = NULL;
 		table->mem_resource[idx].start = 0;
 		table->mem_resource[idx].end = 0;
-		table->mem_resource[idx].flags = IORESOURCE_AUTO | IORESOURCE_UNSET;
+		table->mem_resource[idx].flags = IORESOURCE_AUTO | IORESOURCE_UNSET | IORESOURCE_MEM;
 	}
 }
#8 lore raw

At Sun, 11 Jan 2004 06:33:36 +0100,

Quoted 7 lines · Rene Herman

[1 <text/plain; us-ascii (7bit)>]
Rene Herman wrote:

NOTE: I seem unable to contact Adam Belay; his ISP is not accepting mail from mine. Takashi, if you agree attached patch is a correct fix, could you relay it to Adam?

i forwarded it.

Quoted 4 lines

It also isn't actually an OPL3 issue, but MPU401. Trouble is that sb16.c doesn't set mpu_port to SNDRV_DEFAULT_PORT, but hardcodes the values for the first two cards as 0x330 and 0x300 (Takashi: why is that, by the way? At least for ISA-PnP cards SNDRV_DEFAULT_PORT would seem better?).

yes, SNDRV_DEFAULT_PORT is better. the fixed values are provided for the legacy cards without pnp. i'll fix it.

thanks,

Takashi

#9 lore raw
Quoted 11 lines · Takashi Iwai

At Sun, 11 Jan 2004 06:33:36 +0100,

Quoted 7 lines · Rene Herman

[1 <text/plain; us-ascii (7bit)>]
Rene Herman wrote:

NOTE: I seem unable to contact Adam Belay; his ISP is not accepting mail from mine. Takashi, if you agree attached patch is a correct fix, could you relay it to Adam?

i forwarded it.

I agree with the overall strategy of the patch, but, during testing, I was able to uncover a few bugs introduced by it. I'm reworking how pnp handles flags and should have an updated patch out soon.

Thanks,
Adam

#10 lore raw
Quoted 11 lines · Adam Belay
Quoted 10 lines · Takashi Iwai

At Sun, 11 Jan 2004 06:33:36 +0100,

Quoted 7 lines · Rene Herman

[1 <text/plain; us-ascii (7bit)>]
Rene Herman wrote:

NOTE: I seem unable to contact Adam Belay; his ISP is not accepting mail from mine. Takashi, if you agree attached patch is a correct fix, could you relay it to Adam?

Quoted 5 lines
Quoted 1 line

i forwarded it.

I agree with the overall strategy of the patch, but, during testing, I was able to uncover a few bugs introduced by it. I'm reworking how pnp handles flags and should have an updated patch out soon.

Here's the patch. Any testing would be appreciated.

Thanks,
Adam

drivers/pnp/isapnp/core.c +4 -4
diff -urN a/drivers/pnp/isapnp/core.c b/drivers/pnp/isapnp/core.c
@@ -1039,17 +1039,17 @@
 
 	isapnp_cfg_begin(dev->card->number, dev->number);
 	dev->active = 1;
-	for (tmp = 0; tmp < PNP_MAX_PORT && res->port_resource[tmp].flags & IORESOURCE_IO; tmp++)
+	for (tmp = 0; tmp < PNP_MAX_PORT && !(res->port_resource[tmp].flags & IORESOURCE_UNSET); tmp++)
 		isapnp_write_word(ISAPNP_CFG_PORT+(tmp<<1), res->port_resource[tmp].start);
-	for (tmp = 0; tmp < PNP_MAX_IRQ && res->irq_resource[tmp].flags & IORESOURCE_IRQ; tmp++) {
+	for (tmp = 0; tmp < PNP_MAX_IRQ && !(res->irq_resource[tmp].flags & IORESOURCE_UNSET); tmp++) {
 		int irq = res->irq_resource[tmp].start;
 		if (irq == 2)
 			irq = 9;
 		isapnp_write_byte(ISAPNP_CFG_IRQ+(tmp<<1), irq);
 	}
-	for (tmp = 0; tmp < PNP_MAX_DMA && res->dma_resource[tmp].flags & IORESOURCE_DMA; tmp++)
+	for (tmp = 0; tmp < PNP_MAX_DMA && !(res->dma_resource[tmp].flags & IORESOURCE_UNSET); tmp++)
 		isapnp_write_byte(ISAPNP_CFG_DMA+tmp, res->dma_resource[tmp].start);
-	for (tmp = 0; tmp < PNP_MAX_MEM && res->mem_resource[tmp].flags & IORESOURCE_MEM; tmp++)
+	for (tmp = 0; tmp < PNP_MAX_MEM && !(res->mem_resource[tmp].flags & IORESOURCE_UNSET); tmp++)
 		isapnp_write_word(ISAPNP_CFG_MEM+(tmp<<2), (res->mem_resource[tmp].start >> 8) & 0xffff);
 	/* FIXME: We aren't handling 32bit mems properly here */
 	isapnp_activate(dev->number);
drivers/pnp/manager.c +9 -9
diff -urN a/drivers/pnp/manager.c b/drivers/pnp/manager.c
@@ -223,25 +223,25 @@
 		table->irq_resource[idx].name = NULL;
 		table->irq_resource[idx].start = -1;
 		table->irq_resource[idx].end = -1;
-		table->irq_resource[idx].flags = IORESOURCE_AUTO | IORESOURCE_UNSET;
+		table->irq_resource[idx].flags = IORESOURCE_IRQ | IORESOURCE_AUTO | IORESOURCE_UNSET;
 	}
 	for (idx = 0; idx < PNP_MAX_DMA; idx++) {
 		table->dma_resource[idx].name = NULL;
 		table->dma_resource[idx].start = -1;
 		table->dma_resource[idx].end = -1;
-		table->dma_resource[idx].flags = IORESOURCE_AUTO | IORESOURCE_UNSET;
+		table->dma_resource[idx].flags = IORESOURCE_DMA | IORESOURCE_AUTO | IORESOURCE_UNSET;
 	}
 	for (idx = 0; idx < PNP_MAX_PORT; idx++) {
 		table->port_resource[idx].name = NULL;
 		table->port_resource[idx].start = 0;
 		table->port_resource[idx].end = 0;
-		table->port_resource[idx].flags = IORESOURCE_AUTO | IORESOURCE_UNSET;
+		table->port_resource[idx].flags = IORESOURCE_IO | IORESOURCE_AUTO | IORESOURCE_UNSET;
 	}
 	for (idx = 0; idx < PNP_MAX_MEM; idx++) {
 		table->mem_resource[idx].name = NULL;
 		table->mem_resource[idx].start = 0;
 		table->mem_resource[idx].end = 0;
-		table->mem_resource[idx].flags = IORESOURCE_AUTO | IORESOURCE_UNSET;
+		table->mem_resource[idx].flags = IORESOURCE_MEM | IORESOURCE_AUTO | IORESOURCE_UNSET;
 	}
 }
 

@@ -258,28 +258,28 @@
 			continue;
 		res->irq_resource[idx].start = -1;
 		res->irq_resource[idx].end = -1;
-		res->irq_resource[idx].flags = IORESOURCE_AUTO | IORESOURCE_UNSET;
+		res->irq_resource[idx].flags = IORESOURCE_IRQ | IORESOURCE_AUTO | IORESOURCE_UNSET;
 	}
 	for (idx = 0; idx < PNP_MAX_DMA; idx++) {
 		if (!(res->dma_resource[idx].flags & IORESOURCE_AUTO))
 			continue;
 		res->dma_resource[idx].start = -1;
 		res->dma_resource[idx].end = -1;
-		res->dma_resource[idx].flags = IORESOURCE_AUTO | IORESOURCE_UNSET;
+		res->dma_resource[idx].flags = IORESOURCE_DMA | IORESOURCE_AUTO | IORESOURCE_UNSET;
 	}
 	for (idx = 0; idx < PNP_MAX_PORT; idx++) {
 		if (!(res->port_resource[idx].flags & IORESOURCE_AUTO))
 			continue;
 		res->port_resource[idx].start = 0;
 		res->port_resource[idx].end = 0;
-		res->port_resource[idx].flags = IORESOURCE_AUTO | IORESOURCE_UNSET;
+		res->port_resource[idx].flags = IORESOURCE_IO | IORESOURCE_AUTO | IORESOURCE_UNSET;
 	}
 	for (idx = 0; idx < PNP_MAX_MEM; idx++) {
 		if (!(res->mem_resource[idx].flags & IORESOURCE_AUTO))
 			continue;
 		res->mem_resource[idx].start = 0;
 		res->mem_resource[idx].end = 0;
-		res->mem_resource[idx].flags = IORESOURCE_AUTO | IORESOURCE_UNSET;
+		res->mem_resource[idx].flags = IORESOURCE_MEM | IORESOURCE_AUTO | IORESOURCE_UNSET;
 	}
 }
 

@@ -550,7 +550,7 @@
 {
 	if (resource == NULL)
 		return;
-	resource->flags &= ~IORESOURCE_AUTO;
+	resource->flags &= ~(IORESOURCE_AUTO | IORESOURCE_UNSET);
 	resource->start = start;
 	resource->end = start + size - 1;
 }
drivers/pnp/pnpbios/core.c +1 -1
diff -urN a/drivers/pnp/pnpbios/core.c b/drivers/pnp/pnpbios/core.c
@@ -251,7 +251,7 @@
 	node = pnpbios_kmalloc(node_info.max_node_size, GFP_KERNEL);
 	if (!node)
 		return -1;
-	if (pnp_bios_get_dev_node(&nodenum, (char )PNPMODE_STATIC, node))
+	if (pnp_bios_get_dev_node(&nodenum, (char )PNPMODE_DYNAMIC, node))
 		return -ENODEV;
 	if(pnpbios_write_resources_to_node(res, node)<0) {
 		kfree(node);
drivers/pnp/pnpbios/rsparser.c +4 -4
diff -urN a/drivers/pnp/pnpbios/rsparser.c b/drivers/pnp/pnpbios/rsparser.c
@@ -49,7 +49,7 @@
 pnpbios_parse_allocated_irqresource(struct pnp_resource_table * res, int irq)
 {
 	int i = 0;
-	while ((res->irq_resource[i].flags & IORESOURCE_IRQ) && i < PNP_MAX_IRQ) i++;
+	while (!(res->irq_resource[i].flags & IORESOURCE_UNSET) && i < PNP_MAX_IRQ) i++;
 	if (i < PNP_MAX_IRQ) {
 		res->irq_resource[i].flags = IORESOURCE_IRQ;  // Also clears _UNSET flag
 		if (irq == -1) {

@@ -65,7 +65,7 @@
 pnpbios_parse_allocated_dmaresource(struct pnp_resource_table * res, int dma)
 {
 	int i = 0;
-	while ((res->dma_resource[i].flags & IORESOURCE_DMA) && i < PNP_MAX_DMA) i++;
+	while (!(res->dma_resource[i].flags & IORESOURCE_UNSET) && i < PNP_MAX_DMA) i++;
 	if (i < PNP_MAX_DMA) {
 		res->dma_resource[i].flags = IORESOURCE_DMA;  // Also clears _UNSET flag
 		if (dma == -1) {

@@ -81,7 +81,7 @@
 pnpbios_parse_allocated_ioresource(struct pnp_resource_table * res, int io, int len)
 {
 	int i = 0;
-	while ((res->port_resource[i].flags & IORESOURCE_IO) && i < PNP_MAX_PORT) i++;
+	while (!(res->port_resource[i].flags & IORESOURCE_UNSET) && i < PNP_MAX_PORT) i++;
 	if (i < PNP_MAX_PORT) {
 		res->port_resource[i].flags = IORESOURCE_IO;  // Also clears _UNSET flag
 		if (len <= 0 || (io + len -1) >= 0x10003) {

@@ -97,7 +97,7 @@
 pnpbios_parse_allocated_memresource(struct pnp_resource_table * res, int mem, int len)
 {
 	int i = 0;
-	while ((res->mem_resource[i].flags & IORESOURCE_MEM) && i < PNP_MAX_MEM) i++;
+	while (!(res->mem_resource[i].flags & IORESOURCE_UNSET) && i < PNP_MAX_MEM) i++;
 	if (i < PNP_MAX_MEM) {
 		res->mem_resource[i].flags = IORESOURCE_MEM;  // Also clears _UNSET flag
 		if (len <= 0) {
drivers/pnp/resource.c +11 -8
diff -urN a/drivers/pnp/resource.c b/drivers/pnp/resource.c
@@ -241,6 +241,9 @@
  (*(enda) >= *(startb) && *(enda) <= *(endb)) || \
  (*(starta) < *(startb) && *(enda) > *(endb)))
 
+#define cannot_compare(flags) \
+((flags) & (IORESOURCE_UNSET | IORESOURCE_DISABLED))
+
 int pnp_check_port(struct pnp_dev * dev, int idx)
 {
 	int tmp;

@@ -250,7 +253,7 @@
 	end = &dev->res.port_resource[idx].end;
 
 	/* if the resource doesn't exist, don't complain about it */
-	if (dev->res.port_resource[idx].flags & IORESOURCE_UNSET)
+	if (cannot_compare(dev->res.port_resource[idx].flags))
 		return 1;
 
 	/* check if the resource is already in use, skip if the

@@ -284,7 +287,7 @@
 			continue;
 		for (tmp = 0; tmp < PNP_MAX_PORT; tmp++) {
 			if (tdev->res.port_resource[tmp].flags & IORESOURCE_IO) {
-				if (pnp_port_flags(dev, tmp) & IORESOURCE_DISABLED)
+				if (cannot_compare(tdev->res.port_resource[tmp].flags))
 					continue;
 				tport = &tdev->res.port_resource[tmp].start;
 				tend = &tdev->res.port_resource[tmp].end;

@@ -306,7 +309,7 @@
 	end = &dev->res.mem_resource[idx].end;
 
 	/* if the resource doesn't exist, don't complain about it */
-	if (dev->res.mem_resource[idx].flags & IORESOURCE_UNSET)
+	if (cannot_compare(dev->res.mem_resource[idx].flags))
 		return 1;
 
 	/* check if the resource is already in use, skip if the

@@ -340,7 +343,7 @@
 			continue;
 		for (tmp = 0; tmp < PNP_MAX_MEM; tmp++) {
 			if (tdev->res.mem_resource[tmp].flags & IORESOURCE_MEM) {
-				if (pnp_mem_flags(dev, tmp) & IORESOURCE_DISABLED)
+				if (cannot_compare(tdev->res.mem_resource[tmp].flags))
 					continue;
 				taddr = &tdev->res.mem_resource[tmp].start;
 				tend = &tdev->res.mem_resource[tmp].end;

@@ -365,7 +368,7 @@
 	unsigned long * irq = &dev->res.irq_resource[idx].start;
 
 	/* if the resource doesn't exist, don't complain about it */
-	if (dev->res.irq_resource[idx].flags & IORESOURCE_UNSET)
+	if (cannot_compare(dev->res.irq_resource[idx].flags))
 		return 1;
 
 	/* check if the resource is valid */

@@ -411,7 +414,7 @@
 			continue;
 		for (tmp = 0; tmp < PNP_MAX_IRQ; tmp++) {
 			if (tdev->res.irq_resource[tmp].flags & IORESOURCE_IRQ) {
-				if (pnp_irq_flags(dev, tmp) & IORESOURCE_DISABLED)
+				if (cannot_compare(tdev->res.irq_resource[tmp].flags))
 					continue;
 				if ((tdev->res.irq_resource[tmp].start == *irq))
 					return 0;

@@ -429,7 +432,7 @@
 	unsigned long * dma = &dev->res.dma_resource[idx].start;
 
 	/* if the resource doesn't exist, don't complain about it */
-	if (dev->res.dma_resource[idx].flags & IORESOURCE_UNSET)
+	if (cannot_compare(dev->res.dma_resource[idx].flags))
 		return 1;
 
 	/* check if the resource is valid */

@@ -464,7 +467,7 @@
 			continue;
 		for (tmp = 0; tmp < PNP_MAX_DMA; tmp++) {
 			if (tdev->res.dma_resource[tmp].flags & IORESOURCE_DMA) {
-				if (pnp_dma_flags(dev, tmp) & IORESOURCE_DISABLED)
+				if (cannot_compare(tdev->res.dma_resource[tmp].flags))
 					continue;
 				if ((tdev->res.dma_resource[tmp].start == *dma))
 					return 0;
include/linux/pnp.h +12 -4
@@ -33,7 +33,9 @@
 #define pnp_port_start(dev,bar)   ((dev)->res.port_resource[(bar)].start)
 #define pnp_port_end(dev,bar)     ((dev)->res.port_resource[(bar)].end)
 #define pnp_port_flags(dev,bar)   ((dev)->res.port_resource[(bar)].flags)
-#define pnp_port_valid(dev,bar)   (pnp_port_flags((dev),(bar)) & IORESOURCE_IO)
+#define pnp_port_valid(dev,bar) \
+	((pnp_port_flags((dev),(bar)) & IORESOURCE_IO) && \
+	!(pnp_port_flags((dev),(bar)) & IORESOURCE_UNSET))
 #define pnp_port_len(dev,bar) \
 	((pnp_port_start((dev),(bar)) == 0 &&	\
 	  pnp_port_end((dev),(bar)) ==		\

@@ -45,7 +47,9 @@
 #define pnp_mem_start(dev,bar)   ((dev)->res.mem_resource[(bar)].start)
 #define pnp_mem_end(dev,bar)     ((dev)->res.mem_resource[(bar)].end)
 #define pnp_mem_flags(dev,bar)   ((dev)->res.mem_resource[(bar)].flags)
-#define pnp_mem_valid(dev,bar)   (pnp_mem_flags((dev),(bar)) & IORESOURCE_MEM)
+#define pnp_mem_valid(dev,bar) \
+	((pnp_mem_flags((dev),(bar)) & IORESOURCE_MEM) && \
+	!(pnp_mem_flags((dev),(bar)) & IORESOURCE_UNSET))
 #define pnp_mem_len(dev,bar) \
 	((pnp_mem_start((dev),(bar)) == 0 &&	\
 	  pnp_mem_end((dev),(bar)) ==		\

@@ -56,11 +60,15 @@
 
 #define pnp_irq(dev,bar)	 ((dev)->res.irq_resource[(bar)].start)
 #define pnp_irq_flags(dev,bar)	 ((dev)->res.irq_resource[(bar)].flags)
-#define pnp_irq_valid(dev,bar)   (pnp_irq_flags((dev),(bar)) & IORESOURCE_IRQ)
+#define pnp_irq_valid(dev,bar) \
+	((pnp_irq_flags((dev),(bar)) & IORESOURCE_IRQ) && \
+	!(pnp_irq_flags((dev),(bar)) & IORESOURCE_UNSET))
 
 #define pnp_dma(dev,bar)	 ((dev)->res.dma_resource[(bar)].start)
 #define pnp_dma_flags(dev,bar)	 ((dev)->res.dma_resource[(bar)].flags)
-#define pnp_dma_valid(dev,bar)   (pnp_dma_flags((dev),(bar)) & IORESOURCE_DMA)
+#define pnp_dma_valid(dev,bar) \
+	((pnp_dma_flags((dev),(bar)) & IORESOURCE_DMA) && \
+	!(pnp_dma_flags((dev),(bar)) & IORESOURCE_UNSET))
 
 #define PNP_PORT_FLAG_16BITADDR	(1<<0)
 #define PNP_PORT_FLAG_FIXED	(1<<1)
#11 lore raw
Quoted 5 lines · Adam Belay
Quoted 3 lines

I agree with the overall strategy of the patch, but, during testing, I was able to uncover a few bugs introduced by it. I'm reworking how pnp handles flags and should have an updated patch out soon.

Here's the patch. Any testing would be appreciated.

Yes, works great for the AWE64 OPL3 issue. Many thanks. I'll test this on a few more machines with ISA PnP cards as well.

Rene.

#12 lore raw
In reply to Adam Belay
Quoted 1 line · Adam Belay

Here's the patch. Any testing would be appreciated.

Tested with two more ISA-Pnp soundcards, ES1868 and OPTi 82c933, and their ALSA drivers, snd-es18xx and snd-opti93x, and both work the same as they do without the patch (not quite right that is, but nothing to do with PnP). Also tested with ISA-PnP IDE (on the ES1868), ISA-PnP NE2000 (RTL8019), and ISA-PnP modem. All fine.

Minor point about the patch itself though. In pnp.h, you do:

Quoted 5 lines

-#define pnp_port_valid(dev,bar) (pnp_port_flags((dev),(bar)) & IORESOURCE_IO)
+#define pnp_port_valid(dev,bar) \ + ((pnp_port_flags((dev),(bar)) & IORESOURCE_IO) && \ + !(pnp_port_flags((dev),(bar)) & IORESOURCE_UNSET))

and the same for mem,irq,dma. It seems you could roll these two tests into one with:

#define pnp_port_valid(dev,bar) \
((pnp_port_flags((dev),(bar)) & (IORESOURCE_IO | IORESOURCE_UNSET)) == IORESOURCE_IO)

Basically just an optimisation I guess (just checked and gcc doesn't do this itself) but this also stops the macro arguments from being accessed more than once.

One more point, in isapnp/core.c:isapnp_set_resources()

Review

Quoted 2 lines
(quoted diff) +1 -1
-	for (tmp = 0; tmp < PNP_MAX_PORT && res->port_resource[tmp].flags & IORESOURCE_IO; tmp++)
+	for (tmp = 0; tmp < PNP_MAX_PORT && !(res->port_resource[tmp].flags & IORESOURCE_UNSET); tmp++)

and again same for mem,irq,dma. That is, it goes from only checking IORESOURCE_<TYPE> to only checking IORESOURCE_UNSET. Also checking for the type does sound like a valid sanity check, so would it be better to also check both flags here? Ie:

for (tmp = 0; tmp < PNP_MAX_PORT && (res->port_resource[tmp].flags & (IORESOURCE_IO | IORESOURCE_UNSET)) == IORESOURCE_IO; tmp++)

Incremental patch attached, in case you agree. Compiled, booted and tested.

Rene.

drivers/pnp/isapnp/core.c +4 -4
diff -urN linux-2.6.1-pnp/drivers/pnp/isapnp/core.c linux-2.6.1-pnp-incr/drivers/pnp/isapnp/core.c
@@ -1039,17 +1039,17 @@
 
 	isapnp_cfg_begin(dev->card->number, dev->number);
 	dev->active = 1;
-	for (tmp = 0; tmp < PNP_MAX_PORT && !(res->port_resource[tmp].flags & IORESOURCE_UNSET); tmp++)
+	for (tmp = 0; tmp < PNP_MAX_PORT && (res->port_resource[tmp].flags & (IORESOURCE_IO | IORESOURCE_UNSET)) == IORESOURCE_IO; tmp++)
 		isapnp_write_word(ISAPNP_CFG_PORT+(tmp<<1), res->port_resource[tmp].start);
-	for (tmp = 0; tmp < PNP_MAX_IRQ && !(res->irq_resource[tmp].flags & IORESOURCE_UNSET); tmp++) {
+	for (tmp = 0; tmp < PNP_MAX_IRQ && (res->irq_resource[tmp].flags & (IORESOURCE_IRQ | IORESOURCE_UNSET)) == IORESOURCE_IRQ; tmp++) {
 		int irq = res->irq_resource[tmp].start;
 		if (irq == 2)
 			irq = 9;
 		isapnp_write_byte(ISAPNP_CFG_IRQ+(tmp<<1), irq);
 	}
-	for (tmp = 0; tmp < PNP_MAX_DMA && !(res->dma_resource[tmp].flags & IORESOURCE_UNSET); tmp++)
+	for (tmp = 0; tmp < PNP_MAX_DMA && (res->dma_resource[tmp].flags & (IORESOURCE_DMA | IORESOURCE_UNSET)) == IORESOURCE_DMA; tmp++)
 		isapnp_write_byte(ISAPNP_CFG_DMA+tmp, res->dma_resource[tmp].start);
-	for (tmp = 0; tmp < PNP_MAX_MEM && !(res->mem_resource[tmp].flags & IORESOURCE_UNSET); tmp++)
+	for (tmp = 0; tmp < PNP_MAX_MEM && (res->mem_resource[tmp].flags & (IORESOURCE_MEM | IORESOURCE_UNSET)) == IORESOURCE_MEM; tmp++)
 		isapnp_write_word(ISAPNP_CFG_MEM+(tmp<<2), (res->mem_resource[tmp].start >> 8) & 0xffff);
 	/* FIXME: We aren't handling 32bit mems properly here */
 	isapnp_activate(dev->number);
include/linux/pnp.h +8 -8
diff -urN linux-2.6.1-pnp/include/linux/pnp.h linux-2.6.1-pnp-incr/include/linux/pnp.h
@@ -34,8 +34,8 @@
 #define pnp_port_end(dev,bar)     ((dev)->res.port_resource[(bar)].end)
 #define pnp_port_flags(dev,bar)   ((dev)->res.port_resource[(bar)].flags)
 #define pnp_port_valid(dev,bar) \
-	((pnp_port_flags((dev),(bar)) & IORESOURCE_IO) && \
-	!(pnp_port_flags((dev),(bar)) & IORESOURCE_UNSET))
+	((pnp_port_flags((dev),(bar)) & (IORESOURCE_IO | IORESOURCE_UNSET)) \
+		== IORESOURCE_IO)
 #define pnp_port_len(dev,bar) \
 	((pnp_port_start((dev),(bar)) == 0 &&	\
 	  pnp_port_end((dev),(bar)) ==		\

@@ -48,8 +48,8 @@
 #define pnp_mem_end(dev,bar)     ((dev)->res.mem_resource[(bar)].end)
 #define pnp_mem_flags(dev,bar)   ((dev)->res.mem_resource[(bar)].flags)
 #define pnp_mem_valid(dev,bar) \
-	((pnp_mem_flags((dev),(bar)) & IORESOURCE_MEM) && \
-	!(pnp_mem_flags((dev),(bar)) & IORESOURCE_UNSET))
+	((pnp_mem_flags((dev),(bar)) & (IORESOURCE_MEM | IORESOURCE_UNSET)) \
+		== IORESOURCE_MEM)
 #define pnp_mem_len(dev,bar) \
 	((pnp_mem_start((dev),(bar)) == 0 &&	\
 	  pnp_mem_end((dev),(bar)) ==		\

@@ -61,14 +61,14 @@
 #define pnp_irq(dev,bar)	 ((dev)->res.irq_resource[(bar)].start)
 #define pnp_irq_flags(dev,bar)	 ((dev)->res.irq_resource[(bar)].flags)
 #define pnp_irq_valid(dev,bar) \
-	((pnp_irq_flags((dev),(bar)) & IORESOURCE_IRQ) && \
-	!(pnp_irq_flags((dev),(bar)) & IORESOURCE_UNSET))
+	((pnp_irq_flags((dev),(bar)) & (IORESOURCE_IRQ | IORESOURCE_UNSET)) \
+		== IORESOURCE_IRQ)
 
 #define pnp_dma(dev,bar)	 ((dev)->res.dma_resource[(bar)].start)
 #define pnp_dma_flags(dev,bar)	 ((dev)->res.dma_resource[(bar)].flags)
 #define pnp_dma_valid(dev,bar) \
-	((pnp_dma_flags((dev),(bar)) & IORESOURCE_DMA) && \
-	!(pnp_dma_flags((dev),(bar)) & IORESOURCE_UNSET))
+	((pnp_dma_flags((dev),(bar)) & (IORESOURCE_DMA | IORESOURCE_UNSET)) \
+		== IORESOURCE_DMA)
 
 #define PNP_PORT_FLAG_16BITADDR	(1<<0)
 #define PNP_PORT_FLAG_FIXED	(1<<1)
author
#13 lore raw
In reply to Adam Belay
Quoted 1 line

Here's the patch. Any testing would be appreciated.

This new patch works well for me (SB 16PNP), no problems so far.

Regards...

Signature · 1 line
Manty/BestiaTester -> http://manty.net
#15 lore raw
In reply to Rene Herman

At Sat, 10 Jan 2004 08:24:51 +0100,

Quoted 4 lines · Rene Herman

That is, the driver doesn't complain anymore. Still didn't see the OPL appear in /proc/ioports though (loading with fm_port=0x388 doesn't change that). Haven't investigated further yet, will do.

it's a bug. the attached patch should fix it.

--
Takashi Iwai <[email protected]> ALSA Developer - www.alsa-project.org

sound/isa/sb/sb16.c +1 -1
@@ -483,7 +483,7 @@
 	if (fm_port[dev] > 0 && fm_port[dev] != SNDRV_AUTO_PORT) {
 		if (snd_opl3_create(card, fm_port[dev], fm_port[dev] + 2,
 				    OPL3_HW_OPL3,
-				    fm_port[dev] == port[dev] || fm_port[dev] == 0x388,
+				    acard->fm_res != NULL,
 				    &opl3) < 0) {
 			snd_printk(KERN_ERR PFX "no OPL device at 0x%lx-0x%lx\n",
 				   fm_port[dev], fm_port[dev] + 2);
#16 lore raw

Takashi Iwai wrote:

[ OPl3 not in /proc/ioports for SB16/AWE ]

Quoted 1 line

it's a bug. the attached patch should fix it.

It does:

0388-0389 : OPL2/3 (left)
038a-038b : OPL2/3 (right)

With both patches applied, sb16 is fine again.

Rene.