2003-01-19 17:58:19

by Daniel Ritz

[permalink] [raw]
Subject: [alsa, pnp] more on opl3sa2

more tests with snd-opl3sa2. i have to use the paramers to modprobe since pnp
doesn't work. the reason:
pnpc_register_driver sets driver.bus to its own static struct which doesn't
have a device list. driver_attach tries to scan this device list and compare
with the drivers device list. since there's no one for the bus it fails, the
driver can't see it's devices.

Call stack:
driver_attach
bus_add_driver
driver_register
pnpc_register_driver
alsa_card_opl3sa_init

ok, tried by hand with the config data from bios/lspnp:
# modprobe snd-opl3sa2 isapnp=0 port=0x538 wss_port=0x530 sb_port=0x220 fm_port=0x388 midi_port=0x330 irq=5 dma1=0 dma2=1
Segmentation fault

Unable to handle kernel NULL pointer dereference at virtual address 00000144
printing eip:
d08aecfb
*pde = 00000000
Oops: 0002
CPU: 0
EIP: 0060:[<d08aecfb>] Not tainted
EFLAGS: 00010246
EIP is at snd_opl3sa2_probe+0x3bb/0x3e4 [snd_opl3sa2]
eax: 00000000 ebx: cd9ace00 ecx: d089b734 edx: 00000000
esi: 00000000 edi: cd9ace6c ebp: c7e11f84 esp: c7e11f58
ds: 007b es: 007b ss: 0068
Process modprobe (pid: 1816, threadinfo=c7e10000 task=ce0527e0)
Stack: 00000000 00000000 00000000 00000000 cd9ace24 00000000 00000001 00000000
00000005 c82fec00 c9d5fcc4 c7e11fa8 d08b33c2 00000000 00000000 00000000
00000000 d08b0fe0 c027e304 c027e304 c7e11fbc c01291a2 0804ea38 0804e008
Call Trace:
[<d08b33c2>] alsa_card_opl3sa2_init+0x2a/0xffffe3cc [snd_opl3sa2]
[<d08b0fe0>] +0x0/0x100 [snd_opl3sa2]
[<c01291a2>] sys_init_module+0x116/0x1ac
[<c010a75b>] syscall_call+0x7/0xb

Code: 89 9a 44 01 00 00 8b 45 e8 89 98 60 0c 8b d0 31 c0 eb 0a 89

100% reproducible.

machine is a toshiba tecra 8000 laptop (p3-500), kernel is 2.5.59 + vmlinux.lds.h patch +
sound-firmware-load-fix + jaroslav's driver patch + adam's pnp.h id len patch.

kernel config: no acpi, _all_ pnp options enabled. soundcore compiled in, alsa as module.

# lspci
00:00.0 Host bridge: Intel Corporation 440BX/ZX - 82443BX/ZX Host bridge (AGP disabled) (rev 03)
00:04.0 VGA compatible controller: Neomagic Corporation [MagicMedia 256AV] (rev 12)
00:05.0 Bridge: Intel Corporation 82371AB PIIX4 ISA (rev 02)
00:05.1 IDE interface: Intel Corporation 82371AB PIIX4 IDE (rev 01)
00:05.2 USB Controller: Intel Corporation 82371AB PIIX4 USB (rev 01)
00:05.3 Bridge: Intel Corporation 82371AB PIIX4 ACPI (rev 02)
00:09.0 Communication controller: Toshiba America Info Systems FIR Port (rev 23)
00:0b.0 CardBus bridge: Toshiba America Info Systems ToPIC97 (rev 05)
00:0b.1 CardBus bridge: Toshiba America Info Systems ToPIC97 (rev 05)

# lspnp
01 PNP0c01 memory controller: RAM
02 PNP0200 system peripheral: DMA controller
03 PNP0000 system peripheral: programmable interrupt controller
04 PNP0100 system peripheral: system timer
05 PNP0800 system peripheral: other
06 PNP0c04 reserved: other
07 PNP0303 input device: keyboard
08 PNP0f13 input device: mouse
09 PNP0b00 system peripheral: real time clock
0a PNP0c02 system peripheral: other
0b PNP0700 mass storage device: floppy
0e PNP0501 communications device: RS-232
10 PNP0401 communications device: AT parallel port
11 PNP0a03 bridge controller: PCI
14 PNP0e03 bridge controller: PCMCIA
15 YMH0021 multimedia controller: audio

# lspnp -vv 15
15 YMH0021 multimedia controller: audio
flags: [dynamic]
allocated resources:
io 0x0220-0x0233 [16-bit decode]
io 0x0530-0x0537 [16-bit decode]
io 0x0388-0x038f [16-bit decode]
io 0x0330-0x0333 [16-bit decode]
io 0x0538-0x0539 [16-bit decode]
irq 5 [high edge]
dma 0 [8 bit] [count byte] [compat]
dma 1 [8 bit] [count byte] [compat]
possible resources:
[start dep fn]
io base 0x0220-0x0280 align 0x20 len 0x14 [16-bit decode]
io 0x0530-0x0537 [16-bit decode]
io base 0x0388-0x03b8 align 0x10 len 0x08 [16-bit decode]
io base 0x0300-0x0330 align 0x10 len 0x04 [16-bit decode]
io 0x0538-0x0539 [16-bit decode]
irq mask 0x0ca0 [high edge]
dma mask 0x000b [8 bit] [count byte] [compat]
dma mask 0x000b [8 bit] [count byte] [compat]
[start dep fn]
io base 0x0220-0x0280 align 0x20 len 0x14 [16-bit decode]
io 0x0530-0x0537 [16-bit decode]
io base 0x0388-0x03b8 align 0x10 len 0x08 [16-bit decode]
io base 0x0300-0x0330 align 0x10 len 0x04 [16-bit decode]
io 0x0538-0x0539 [16-bit decode]
irq mask 0x0ca0 [high edge]
dma mask 0x000b [8 bit] [count byte] [compat]
dma disabled [8 bit] [count byte] [compat]
[start dep fn]
io base 0x0220-0x0280 align 0x20 len 0x14 [16-bit decode]
io 0x0540-0x0547 [16-bit decode]
io base 0x0388-0x03b8 align 0x10 len 0x08 [16-bit decode]
io base 0x0300-0x0330 align 0x10 len 0x04 [16-bit decode]
io 0x0548-0x0549 [16-bit decode]
irq mask 0x0ca0 [high edge]
dma mask 0x000b [8 bit] [count byte] [compat]
dma mask 0x000b [8 bit] [count byte] [compat]
[start dep fn]
io base 0x0220-0x0280 align 0x20 len 0x14 [16-bit decode]
io 0x0540-0x0547 [16-bit decode]
io base 0x0388-0x03b8 align 0x10 len 0x08 [16-bit decode]
io base 0x0300-0x0330 align 0x10 len 0x04 [16-bit decode]
io 0x0548-0x0549 [16-bit decode]
irq mask 0x0ca0 [high edge]
dma mask 0x000b [8 bit] [count byte] [compat]
dma disabled [8 bit] [count byte] [compat]
[start dep fn]
io base 0x0220-0x0280 align 0x20 len 0x14 [16-bit decode]
io 0x0550-0x0557 [16-bit decode]
io base 0x0388-0x03b8 align 0x10 len 0x08 [16-bit decode]
io base 0x0300-0x0330 align 0x10 len 0x04 [16-bit decode]
io 0x0558-0x0559 [16-bit decode]
irq mask 0x0ca0 [high edge]
dma mask 0x000b [8 bit] [count byte] [compat]
dma mask 0x000b [8 bit] [count byte] [compat]
[start dep fn]
io base 0x0220-0x0280 align 0x20 len 0x14 [16-bit decode]
io 0x0550-0x0557 [16-bit decode]
io base 0x0388-0x03b8 align 0x10 len 0x08 [16-bit decode]
io base 0x0300-0x0330 align 0x10 len 0x04 [16-bit decode]
io 0x0558-0x0559 [16-bit decode]
irq mask 0x0ca0 [high edge]
dma mask 0x000b [8 bit] [count byte] [compat]
dma disabled [8 bit] [count byte] [compat]
[start dep fn]
io base 0x0220-0x0280 align 0x20 len 0x14 [16-bit decode]
io 0x0560-0x0567 [16-bit decode]
io base 0x0388-0x03b8 align 0x10 len 0x08 [16-bit decode]
io base 0x0300-0x0330 align 0x10 len 0x04 [16-bit decode]
io 0x0568-0x0569 [16-bit decode]
irq mask 0x0ca0 [high edge]
dma mask 0x000b [8 bit] [count byte] [compat]
dma mask 0x000b [8 bit] [count byte] [compat]
[start dep fn]
io base 0x0220-0x0280 align 0x20 len 0x14 [16-bit decode]
io 0x0560-0x0567 [16-bit decode]
io base 0x0388-0x03b8 align 0x10 len 0x08 [16-bit decode]
io base 0x0300-0x0330 align 0x10 len 0x04 [16-bit decode]
io 0x0568-0x0569 [16-bit decode]
irq mask 0x0ca0 [high edge]
dma mask 0x000b [8 bit] [count byte] [compat]
dma disabled [8 bit] [count byte] [compat]
[end dep fn]
compatible devices:
identifier 'OPL3-SA3 Sound System'


if you need more info or if you want me to test patches...just ask

rgds,
-daniel


2003-01-21 18:57:06

by Daniel Ritz

[permalink] [raw]
Subject: Re: [alsa, pnp] more on opl3sa2

ok, found that one...very simple:
against 2.5.59 + jaroslav's driver patch...


--- linux-2.5/sound/isa/opl3sa2.c~ 2003-01-21 20:01:10.000000000 +0100
+++ linux-2.5/sound/isa/opl3sa2.c 2003-01-21 19:58:06.000000000 +0100
@@ -830,7 +830,8 @@
if ((err = snd_card_register(card)) < 0)
goto __error;

- pnpc_set_drvdata(pcard, card);
+ if (pcard)
+ pnpc_set_drvdata(pcard, card);
snd_opl3sa2_cards[dev] = card;
return 0;


the card is not detected by pnp, that problem stays. is that a problem of the pnp layer or is
my toshiba laptop just so damn stupid??

rgds
-daniel



On Sunday 19 January 2003 19:07, Daniel Ritz wrote:
> more tests with snd-opl3sa2. i have to use the paramers to modprobe since
> pnp doesn't work. the reason:
> pnpc_register_driver sets driver.bus to its own static struct which doesn't
> have a device list. driver_attach tries to scan this device list and
> compare with the drivers device list. since there's no one for the bus it
> fails, the driver can't see it's devices.
>
> Call stack:
> driver_attach
> bus_add_driver
> driver_register
> pnpc_register_driver
> alsa_card_opl3sa_init
>
> ok, tried by hand with the config data from bios/lspnp:
> # modprobe snd-opl3sa2 isapnp=0 port=0x538 wss_port=0x530 sb_port=0x220
> fm_port=0x388 midi_port=0x330 irq=5 dma1=0 dma2=1 Segmentation fault
>
> Unable to handle kernel NULL pointer dereference at virtual address
> 00000144 printing eip:
> d08aecfb
> *pde = 00000000
> Oops: 0002
> CPU: 0
> EIP: 0060:[<d08aecfb>] Not tainted
> EFLAGS: 00010246
> EIP is at snd_opl3sa2_probe+0x3bb/0x3e4 [snd_opl3sa2]
> eax: 00000000 ebx: cd9ace00 ecx: d089b734 edx: 00000000
> esi: 00000000 edi: cd9ace6c ebp: c7e11f84 esp: c7e11f58
> ds: 007b es: 007b ss: 0068
> Process modprobe (pid: 1816, threadinfo=c7e10000 task=ce0527e0)
> Stack: 00000000 00000000 00000000 00000000 cd9ace24 00000000 00000001
> 00000000 00000005 c82fec00 c9d5fcc4 c7e11fa8 d08b33c2 00000000 00000000
> 00000000 00000000 d08b0fe0 c027e304 c027e304 c7e11fbc c01291a2 0804ea38
> 0804e008 Call Trace:
> [<d08b33c2>] alsa_card_opl3sa2_init+0x2a/0xffffe3cc [snd_opl3sa2]
> [<d08b0fe0>] +0x0/0x100 [snd_opl3sa2]
> [<c01291a2>] sys_init_module+0x116/0x1ac
> [<c010a75b>] syscall_call+0x7/0xb
>
> Code: 89 9a 44 01 00 00 8b 45 e8 89 98 60 0c 8b d0 31 c0 eb 0a 89
>
> 100% reproducible.
>
> machine is a toshiba tecra 8000 laptop (p3-500), kernel is 2.5.59 +
> vmlinux.lds.h patch + sound-firmware-load-fix + jaroslav's driver patch +
> adam's pnp.h id len patch.
>
> kernel config: no acpi, _all_ pnp options enabled. soundcore compiled in,
> alsa as module.
>
> # lspci
> 00:00.0 Host bridge: Intel Corporation 440BX/ZX - 82443BX/ZX Host bridge
> (AGP disabled) (rev 03) 00:04.0 VGA compatible controller: Neomagic
> Corporation [MagicMedia 256AV] (rev 12) 00:05.0 Bridge: Intel Corporation
> 82371AB PIIX4 ISA (rev 02)
> 00:05.1 IDE interface: Intel Corporation 82371AB PIIX4 IDE (rev 01)
> 00:05.2 USB Controller: Intel Corporation 82371AB PIIX4 USB (rev 01)
> 00:05.3 Bridge: Intel Corporation 82371AB PIIX4 ACPI (rev 02)
> 00:09.0 Communication controller: Toshiba America Info Systems FIR Port
> (rev 23) 00:0b.0 CardBus bridge: Toshiba America Info Systems ToPIC97 (rev
> 05) 00:0b.1 CardBus bridge: Toshiba America Info Systems ToPIC97 (rev 05)
>
> # lspnp
> 01 PNP0c01 memory controller: RAM
> 02 PNP0200 system peripheral: DMA controller
> 03 PNP0000 system peripheral: programmable interrupt controller
> 04 PNP0100 system peripheral: system timer
> 05 PNP0800 system peripheral: other
> 06 PNP0c04 reserved: other
> 07 PNP0303 input device: keyboard
> 08 PNP0f13 input device: mouse
> 09 PNP0b00 system peripheral: real time clock
> 0a PNP0c02 system peripheral: other
> 0b PNP0700 mass storage device: floppy
> 0e PNP0501 communications device: RS-232
> 10 PNP0401 communications device: AT parallel port
> 11 PNP0a03 bridge controller: PCI
> 14 PNP0e03 bridge controller: PCMCIA
> 15 YMH0021 multimedia controller: audio
>
> # lspnp -vv 15
> 15 YMH0021 multimedia controller: audio
> flags: [dynamic]
> allocated resources:
> io 0x0220-0x0233 [16-bit decode]
> io 0x0530-0x0537 [16-bit decode]
> io 0x0388-0x038f [16-bit decode]
> io 0x0330-0x0333 [16-bit decode]
> io 0x0538-0x0539 [16-bit decode]
> irq 5 [high edge]
> dma 0 [8 bit] [count byte] [compat]
> dma 1 [8 bit] [count byte] [compat]
> possible resources:
> [start dep fn]
> io base 0x0220-0x0280 align 0x20 len 0x14 [16-bit decode]
> io 0x0530-0x0537 [16-bit decode]
> io base 0x0388-0x03b8 align 0x10 len 0x08 [16-bit decode]
> io base 0x0300-0x0330 align 0x10 len 0x04 [16-bit decode]
> io 0x0538-0x0539 [16-bit decode]
> irq mask 0x0ca0 [high edge]
> dma mask 0x000b [8 bit] [count byte] [compat]
> dma mask 0x000b [8 bit] [count byte] [compat]
> [start dep fn]
> io base 0x0220-0x0280 align 0x20 len 0x14 [16-bit decode]
> io 0x0530-0x0537 [16-bit decode]
> io base 0x0388-0x03b8 align 0x10 len 0x08 [16-bit decode]
> io base 0x0300-0x0330 align 0x10 len 0x04 [16-bit decode]
> io 0x0538-0x0539 [16-bit decode]
> irq mask 0x0ca0 [high edge]
> dma mask 0x000b [8 bit] [count byte] [compat]
> dma disabled [8 bit] [count byte] [compat]
> [start dep fn]
> io base 0x0220-0x0280 align 0x20 len 0x14 [16-bit decode]
> io 0x0540-0x0547 [16-bit decode]
> io base 0x0388-0x03b8 align 0x10 len 0x08 [16-bit decode]
> io base 0x0300-0x0330 align 0x10 len 0x04 [16-bit decode]
> io 0x0548-0x0549 [16-bit decode]
> irq mask 0x0ca0 [high edge]
> dma mask 0x000b [8 bit] [count byte] [compat]
> dma mask 0x000b [8 bit] [count byte] [compat]
> [start dep fn]
> io base 0x0220-0x0280 align 0x20 len 0x14 [16-bit decode]
> io 0x0540-0x0547 [16-bit decode]
> io base 0x0388-0x03b8 align 0x10 len 0x08 [16-bit decode]
> io base 0x0300-0x0330 align 0x10 len 0x04 [16-bit decode]
> io 0x0548-0x0549 [16-bit decode]
> irq mask 0x0ca0 [high edge]
> dma mask 0x000b [8 bit] [count byte] [compat]
> dma disabled [8 bit] [count byte] [compat]
> [start dep fn]
> io base 0x0220-0x0280 align 0x20 len 0x14 [16-bit decode]
> io 0x0550-0x0557 [16-bit decode]
> io base 0x0388-0x03b8 align 0x10 len 0x08 [16-bit decode]
> io base 0x0300-0x0330 align 0x10 len 0x04 [16-bit decode]
> io 0x0558-0x0559 [16-bit decode]
> irq mask 0x0ca0 [high edge]
> dma mask 0x000b [8 bit] [count byte] [compat]
> dma mask 0x000b [8 bit] [count byte] [compat]
> [start dep fn]
> io base 0x0220-0x0280 align 0x20 len 0x14 [16-bit decode]
> io 0x0550-0x0557 [16-bit decode]
> io base 0x0388-0x03b8 align 0x10 len 0x08 [16-bit decode]
> io base 0x0300-0x0330 align 0x10 len 0x04 [16-bit decode]
> io 0x0558-0x0559 [16-bit decode]
> irq mask 0x0ca0 [high edge]
> dma mask 0x000b [8 bit] [count byte] [compat]
> dma disabled [8 bit] [count byte] [compat]
> [start dep fn]
> io base 0x0220-0x0280 align 0x20 len 0x14 [16-bit decode]
> io 0x0560-0x0567 [16-bit decode]
> io base 0x0388-0x03b8 align 0x10 len 0x08 [16-bit decode]
> io base 0x0300-0x0330 align 0x10 len 0x04 [16-bit decode]
> io 0x0568-0x0569 [16-bit decode]
> irq mask 0x0ca0 [high edge]
> dma mask 0x000b [8 bit] [count byte] [compat]
> dma mask 0x000b [8 bit] [count byte] [compat]
> [start dep fn]
> io base 0x0220-0x0280 align 0x20 len 0x14 [16-bit decode]
> io 0x0560-0x0567 [16-bit decode]
> io base 0x0388-0x03b8 align 0x10 len 0x08 [16-bit decode]
> io base 0x0300-0x0330 align 0x10 len 0x04 [16-bit decode]
> io 0x0568-0x0569 [16-bit decode]
> irq mask 0x0ca0 [high edge]
> dma mask 0x000b [8 bit] [count byte] [compat]
> dma disabled [8 bit] [count byte] [compat]
> [end dep fn]
> compatible devices:
> identifier 'OPL3-SA3 Sound System'
>
>
> if you need more info or if you want me to test patches...just ask
>
> rgds,
> -daniel

2003-01-21 20:00:32

by Jaroslav Kysela

[permalink] [raw]
Subject: Re: [alsa, pnp] more on opl3sa2

On Tue, 21 Jan 2003, Daniel Ritz wrote:

> ok, found that one...very simple:
> against 2.5.59 + jaroslav's driver patch...
>
>
> --- linux-2.5/sound/isa/opl3sa2.c~ 2003-01-21 20:01:10.000000000 +0100
> +++ linux-2.5/sound/isa/opl3sa2.c 2003-01-21 19:58:06.000000000 +0100
> @@ -830,7 +830,8 @@
> if ((err = snd_card_register(card)) < 0)
> goto __error;
>
> - pnpc_set_drvdata(pcard, card);
> + if (pcard)
> + pnpc_set_drvdata(pcard, card);
> snd_opl3sa2_cards[dev] = card;
> return 0;
>
>
> the card is not detected by pnp, that problem stays. is that a problem of the pnp layer or is
> my toshiba laptop just so damn stupid??

Nope. It's fault of the driver. It scans for a card. Actually, the
structure card -> devices is created only by the ISA PnP driver.

I don't see any reason to not group the PnP BIOS devices into one "card",
too. Adam, do you have any comments?

Here is the patch for PnP BIOS and opl3sa2 - note that the only difference
for opl3sa2 driver is line:

+ /* Yamaha OPL3-SA3 (PnP BIOS) */
+ {.id = "PnPbios", .driver_data = 0, .devs = { {.id="YMH0021"}, }

Jaroslav

===== core.c 1.21 vs edited =====
--- 1.21/drivers/pnp/pnpbios/core.c Sun Jan 12 21:14:10 2003
+++ edited/core.c Tue Jan 21 21:05:39 2003
@@ -1387,7 +1387,11 @@
.disable = pnpbios_disable_resources,
};

+#ifdef CONFIG_PNP_CARD
+static inline int insert_device(struct pnp_card *card, struct pnp_dev *dev)
+#else
static inline int insert_device(struct pnp_dev *dev)
+#endif
{
struct list_head * pos;
struct pnp_dev * pnp_dev;
@@ -1396,7 +1400,11 @@
if (dev->number == pnp_dev->number)
return -1;
}
+#ifdef CONFIG_PNP_CARD
+ pnpc_add_device(card, dev);
+#else
pnp_add_device(dev);
+#endif
return 0;
}

@@ -1411,6 +1419,7 @@
struct pnp_dev_node_info node_info;
struct pnp_dev *dev;
struct pnp_id *dev_id;
+ struct pnp_card *card;

if (!pnp_bios_present())
return;
@@ -1421,6 +1430,28 @@
node = pnpbios_kmalloc(node_info.max_node_size, GFP_KERNEL);
if (!node)
return;
+
+#ifdef CONFIG_PNP_CARD
+ card = pnpbios_kmalloc(sizeof(struct pnp_card), GFP_KERNEL);
+ if (!card) {
+ kfree(node);
+ return;
+ }
+ memset(card, 0, sizeof(struct pnp_card));
+
+ dev_id = pnpbios_kmalloc(sizeof(struct pnp_id), GFP_KERNEL);
+ if (!dev_id) {
+ kfree(node);
+ kfree(card);
+ return;
+ }
+ memset(dev_id, 0, sizeof(struct pnp_id));
+
+ strcpy(dev_id->id, "PnPbios");
+ INIT_LIST_HEAD(&card->devices);
+ pnpc_add_id(dev_id, card);
+ card->protocol = &pnpbios_protocol;
+#endif

for(nodenum=0; nodenum<0xff; ) {
u8 thisnodenum = nodenum;
@@ -1444,10 +1475,11 @@
break;
}
memset(dev_id,0,sizeof(struct pnp_id));
+ dev->card = card;
dev->number = thisnodenum;
strcpy(dev->name,"Unknown Device");
pnpid32_to_pnpid(node->eisa_id,id);
- memcpy(dev_id->id,id,7);
+ memcpy(dev_id->id,id,sizeof(dev_id->id));
pnp_add_id(dev_id, dev);
pos = node_current_resource_data_to_dev(node,dev);
pos = node_possible_resource_data_to_dev(pos,node,dev);
@@ -1465,7 +1497,11 @@

dev->protocol = &pnpbios_protocol;

+#ifdef CONFIG_PNP_CARD
+ if(insert_device(card, dev)<0) {
+#else
if(insert_device(dev)<0) {
+#endif
kfree(dev_id);
kfree(dev);
} else
@@ -1476,6 +1512,9 @@
}
}
kfree(node);
+#ifdef CONFIG_PNP_CARD
+ pnpc_add_card(card);
+#endif

printk(KERN_INFO "PnPBIOS: %i node%s reported by PnP BIOS; %i recorded by driver\n",
nodes_got, nodes_got != 1 ? "s" : "", devs);
===== opl3sa2.c 1.14 vs edited =====
--- 1.14/sound/isa/opl3sa2.c Sat Dec 7 17:11:30 2002
+++ edited/opl3sa2.c Tue Jan 21 20:37:54 2003
@@ -134,6 +134,7 @@

struct snd_opl3sa2 {
snd_card_t *card;
+ int dev_no; /* device / slot index */
int version; /* 2 or 3 */
unsigned long port; /* control port */
struct resource *res_port; /* control port resource */
@@ -161,23 +162,22 @@

#ifdef CONFIG_PNP

-static struct pnp_card *snd_opl3sa2_isapnp_cards[SNDRV_CARDS] = SNDRV_DEFAULT_PTR;
-static const struct pnp_card_id *snd_opl3sa2_isapnp_id[SNDRV_CARDS] = SNDRV_DEFAULT_PTR;
-
-static struct pnp_card_id snd_opl3sa2_pnpids[] = {
+static struct pnp_card_device_id snd_opl3sa2_pnpids[] = {
/* Yamaha YMF719E-S (Genius Sound Maker 3DX) */
- {.id = "YMH0020", .driver_data = 0, devs : { {.id="YMH0021"}, } },
+ {.id = "YMH0020", .driver_data = 0, .devs = { {.id="YMH0021"}, } },
/* Yamaha OPL3-SA3 (integrated on Intel's Pentium II AL440LX motherboard) */
- {.id = "YMH0030", .driver_data = 0, devs : { {.id="YMH0021"}, } },
+ {.id = "YMH0030", .driver_data = 0, .devs = { {.id="YMH0021"}, } },
/* Yamaha OPL3-SA2 */
- {.id = "YMH0800", .driver_data = 0, devs : { {.id="YMH0021"}, } },
+ {.id = "YMH0800", .driver_data = 0, .devs = { {.id="YMH0021"}, } },
/* NeoMagic MagicWave 3DX */
- {.id = "NMX2200", .driver_data = 0, devs : { {.id="NMX2210"}, } },
+ {.id = "NMX2200", .driver_data = 0, .devs = { {.id="NMX2210"}, } },
+ /* Yamaha OPL3-SA3 (PnP BIOS) */
+ {.id = "PnPbios", .driver_data = 0, .devs = { {.id="YMH0021"}, } },
/* --- */
{.id = "", } /* end */
};

-/*PNP_CARD_TABLE(snd_opl3sa2_pnpids);*/
+MODULE_DEVICE_TABLE(pnp_card, snd_opl3sa2_pnpids);

#endif /* CONFIG_PNP */

@@ -625,40 +625,69 @@
#endif /* CONFIG_PM */

#ifdef CONFIG_PNP
-static int __init snd_opl3sa2_isapnp(int dev, opl3sa2_t *chip)
+static int __devinit snd_opl3sa2_isapnp(int dev, opl3sa2_t *chip,
+ struct pnp_card *card,
+ const struct pnp_card_device_id *id)
{
- const struct pnp_card_id *id = snd_opl3sa2_isapnp_id[dev];
- struct pnp_card *card = snd_opl3sa2_isapnp_cards[dev];
struct pnp_dev *pdev;
+ struct pnp_res_cfg cfg;
+ int err;

- chip->dev = pnp_request_card_device(card, id->devs[0].id, NULL);
- pdev = chip->dev;
+ pdev = chip->dev = pnp_request_card_device(card, id->devs[0].id, NULL);
if (!pdev){
snd_printdd("isapnp OPL3-SA: a card was found but it did not contain the needed devices\n");
return -ENODEV;
}
- sb_port[dev] = pdev->resource[0].start;
- wss_port[dev] = pdev->resource[1].start;
- fm_port[dev] = pdev->resource[2].start;
- midi_port[dev] = pdev->resource[3].start;
- port[dev] = pdev->resource[4].start;
- dma1[dev] = pdev->dma_resource[0].start;
- dma2[dev] = pdev->dma_resource[1].start;
- irq[dev] = pdev->irq_resource[0].start;
+ if (pnp_init_res_cfg(&cfg) < 0)
+ return -ENOMEM;
+ /* override resources */
+ if (sb_port[dev] != SNDRV_AUTO_PORT)
+ pnp_resource_change(&cfg.io_resource[0], sb_port[dev], 16);
+ if (wss_port[dev] != SNDRV_AUTO_PORT)
+ pnp_resource_change(&cfg.io_resource[1], wss_port[dev], 8);
+ if (fm_port[dev] != SNDRV_AUTO_PORT)
+ pnp_resource_change(&cfg.io_resource[2], fm_port[dev], 4);
+ if (midi_port[dev] != SNDRV_AUTO_PORT)
+ pnp_resource_change(&cfg.io_resource[3], midi_port[dev], 2);
+ if (port[dev] != SNDRV_AUTO_PORT)
+ pnp_resource_change(&cfg.io_resource[4], port[dev], 2);
+ if (dma1[dev] != SNDRV_AUTO_DMA)
+ pnp_resource_change(&cfg.dma_resource[0], dma1[dev], 1);
+ if (dma2[dev] != SNDRV_AUTO_DMA)
+ pnp_resource_change(&cfg.dma_resource[1], dma2[dev], 1);
+ if (irq[dev] != SNDRV_AUTO_IRQ)
+ pnp_resource_change(&cfg.irq_resource[0], irq[dev], 1);
+ err = pnp_activate_dev(pdev, &cfg);
+ if (err < 0) {
+ snd_printk(KERN_ERR "cannot activate pnp device (out of resources?)\n");
+ return err;
+ }
+ sb_port[dev] = pnp_port_start(pdev, 0);
+ wss_port[dev] = pnp_port_start(pdev, 1);
+ fm_port[dev] = pnp_port_start(pdev, 2);
+ midi_port[dev] = pnp_port_start(pdev, 3);
+ port[dev] = pnp_port_start(pdev, 4);
+ dma1[dev] = pnp_dma(pdev, 0);
+ dma2[dev] = pnp_dma(pdev, 1);
+ irq[dev] = pnp_irq(pdev, 0);
snd_printdd("isapnp OPL3-SA: sb port=0x%lx, wss port=0x%lx, fm port=0x%lx, midi port=0x%lx\n",
sb_port[dev], wss_port[dev], fm_port[dev], midi_port[dev]);
snd_printdd("isapnp OPL3-SA: control port=0x%lx, dma1=%i, dma2=%i, irq=%i\n",
port[dev], dma1[dev], dma2[dev], irq[dev]);
return 0;
}
-
+#else
+static int __devinit snd_opl3sa2_isapnp(int dev, opl3sa2_t *chip,
+ struct pnp_card *card,
+ const struct pnp_card_device_id *id)
+{
+ return -ENODEV;
+}
#endif /* CONFIG_PNP */

static int snd_opl3sa2_free(opl3sa2_t *chip)
{
-#ifdef CONFIG_PNP
- chip->dev = NULL;
-#endif
+ snd_opl3sa2_cards[chip->dev_no] = NULL;
#ifdef CONFIG_PM
if (chip->pm_dev)
pm_unregister(chip->pm_dev);
@@ -679,7 +708,9 @@
return snd_opl3sa2_free(chip);
}

-static int __init snd_opl3sa2_probe(int dev)
+static int __devinit snd_opl3sa2_probe(int dev, int _isapnp,
+ struct pnp_card *pcard,
+ const struct pnp_card_device_id *pid)
{
int xirq, xdma1, xdma2;
snd_card_t *card;
@@ -691,9 +722,7 @@
};
int err;

-#ifdef CONFIG_PNP
- if (!isapnp[dev]) {
-#endif
+ if (!_isapnp) {
if (port[dev] == SNDRV_AUTO_PORT) {
snd_printk("specify port\n");
return -EINVAL;
@@ -710,9 +739,7 @@
snd_printk("specify midi_port\n");
return -EINVAL;
}
-#ifdef CONFIG_PNP
}
-#endif
card = snd_card_new(index[dev], id[dev], THIS_MODULE, 0);
if (card == NULL)
return -ENOMEM;
@@ -723,13 +750,12 @@
err = -ENOMEM;
goto __error;
}
+ chip->dev_no = dev;
chip->irq = -1;
if ((err = snd_device_new(card, SNDRV_DEV_LOWLEVEL, chip, &ops)) < 0)
goto __error;
-#ifdef CONFIG_PNP
- if (isapnp[dev] && (err = snd_opl3sa2_isapnp(dev, chip)) < 0)
+ if (_isapnp && (err = snd_opl3sa2_isapnp(dev, chip, pcard, pid)) < 0)
goto __error;
-#endif
chip->ymode = opl3sa3_ymode[dev] & 0x03 ; /* initialise this card from supplied (or default) parameter*/
chip->card = card;
chip->port = port[dev];
@@ -806,6 +832,8 @@
if ((err = snd_card_register(card)) < 0)
goto __error;

+ if (pcard)
+ pnpc_set_drvdata(pcard, card);
snd_opl3sa2_cards[dev] = card;
return 0;

@@ -815,18 +843,16 @@
}

#ifdef CONFIG_PNP
-static int __init snd_opl3sa2_isapnp_detect(struct pnp_card *card,
- const struct pnp_card_id *id)
+static int __devinit snd_opl3sa2_isapnp_detect(struct pnp_card *card,
+ const struct pnp_card_device_id *id)
{
static int dev;
int res;

for ( ; dev < SNDRV_CARDS; dev++) {
- if (!enable[dev])
+ if (!enable[dev] || !isapnp[dev])
continue;
- snd_opl3sa2_isapnp_cards[dev] = card;
- snd_opl3sa2_isapnp_id[dev] = id;
- res = snd_opl3sa2_probe(dev);
+ res = snd_opl3sa2_probe(dev, 1, card, id);
if (res < 0)
return res;
dev++;
@@ -835,18 +861,21 @@
return -ENODEV;
}

-static void snd_opl3sa2_isapnp_remove(struct pnp_card * card)
+static void __devexit snd_opl3sa2_isapnp_remove(struct pnp_card * pcard)
{
-/* FIXME */
+ snd_card_t * card = (snd_card_t *) pnpc_get_drvdata(pcard);
+
+ snd_card_disconnect(card);
+ snd_card_free_in_thread(card);
}

static struct pnpc_driver opl3sa2_pnpc_driver = {
+ .flags = PNPC_DRIVER_DO_NOT_ACTIVATE,
.name = "opl3sa2",
.id_table = snd_opl3sa2_pnpids,
.probe = snd_opl3sa2_isapnp_detect,
- .remove = snd_opl3sa2_isapnp_remove,
+ .remove = __devexit_p(snd_opl3sa2_isapnp_remove),
};
-
#endif /* CONFIG_PNP */

static int __init alsa_card_opl3sa2_init(void)
@@ -860,7 +889,7 @@
if (isapnp[dev])
continue;
#endif
- if (snd_opl3sa2_probe(dev) >= 0)
+ if (snd_opl3sa2_probe(dev, 0, NULL, NULL) >= 0)
cards++;
}
#ifdef CONFIG_PNP
@@ -870,6 +899,9 @@
#ifdef MODULE
printk(KERN_ERR "Yamaha OPL3-SA soundcard not found or device busy\n");
#endif
+#ifdef CONFIG_PNP
+ pnpc_unregister_driver(&opl3sa2_pnpc_driver);
+#endif
return -ENODEV;
}
return 0;
@@ -877,10 +909,13 @@

static void __exit alsa_card_opl3sa2_exit(void)
{
- int idx;
+ int dev;

- for (idx = 0; idx < SNDRV_CARDS; idx++)
- snd_card_free(snd_opl3sa2_cards[idx]);
+#ifdef CONFIG_PNP
+ pnpc_unregister_driver(&opl3sa2_pnpc_driver);
+#endif
+ for (dev = 0; dev < SNDRV_CARDS; dev++)
+ snd_card_free(snd_opl3sa2_cards[dev]);
}

module_init(alsa_card_opl3sa2_init)

-----
Jaroslav Kysela <[email protected]>
Linux Kernel Sound Maintainer
ALSA Project, SuSE Labs

2003-01-21 20:51:25

by Adam Belay

[permalink] [raw]
Subject: Re: [alsa, pnp] more on opl3sa2

On Tue, Jan 21, 2003 at 09:09:14PM +0100, Jaroslav Kysela wrote:

> > the card is not detected by pnp, that problem stays. is that a problem of the pnp layer or is
> > my toshiba laptop just so damn stupid??
>
> Nope. It's fault of the driver. It scans for a card. Actually, the
> structure card -> devices is created only by the ISA PnP driver.
>
> I don't see any reason to not group the PnP BIOS devices into one "card",
> too. Adam, do you have any comments?
>

I have considered this approach several times. However, there are the following
problems with representing the pnpbios devices under one card:

1.) If a driver attaches to the pnpbios card all other card-based drivers will
be unable to use the pnpbios. One will attach and cause the others to fail. It
is possible for the user to have more than one pnpbios sound card but with this
approach such a user would only be able to use one sound device from the entire
pnpbios.

2.) Doing so would misrepresent the pnpbios topology because it physically
doesn't have any cards.

3.) The opl3sa2 driver doesn't need a card because it is only asking for one
device anyway. Using the card interface puts unnecessary overhead on both the
driver and the pnp layer.

4.) The PnP Card Services were designed to support older hardware (pre-pnpbios).
Eventually vendors realized how stupid it was to use more than one device to
perform a single function. Therefore the opl3sa2 and others do not use several
devices and do not need a card interface.

I propose that we use a different solution...

What if we have the opl3sa2 driver along with the other one device drivers
use the pnp_driver interface instead of the pnpc_driver. Jaroslav, Do you
agree with this or would you suggest something else? If you agree, I'll get
to work on a patch.

Regards,
Adam

2003-01-21 21:36:05

by Jaroslav Kysela

[permalink] [raw]
Subject: Re: [alsa, pnp] more on opl3sa2

On Tue, 21 Jan 2003, Adam Belay wrote:

> On Tue, Jan 21, 2003 at 09:09:14PM +0100, Jaroslav Kysela wrote:
>
> > > the card is not detected by pnp, that problem stays. is that a problem of the pnp layer or is
> > > my toshiba laptop just so damn stupid??
> >
> > Nope. It's fault of the driver. It scans for a card. Actually, the
> > structure card -> devices is created only by the ISA PnP driver.
> >
> > I don't see any reason to not group the PnP BIOS devices into one "card",
> > too. Adam, do you have any comments?
> >
>
> I have considered this approach several times. However, there are the following
> problems with representing the pnpbios devices under one card:
>
> 1.) If a driver attaches to the pnpbios card all other card-based drivers will
> be unable to use the pnpbios. One will attach and cause the others to fail. It
> is possible for the user to have more than one pnpbios sound card but with this
> approach such a user would only be able to use one sound device from the entire
> pnpbios.

I see. I think it's a design problem then. The rule card -> one driver is
bad. We need something between card and device which will take care about
drivers. Unfortunately, this information is dynamic (only driver knows
which devices have to be attached).

I think that we need to discuss this thing very carefully.

> 2.) Doing so would misrepresent the pnpbios topology because it physically
> doesn't have any cards.
>
> 3.) The opl3sa2 driver doesn't need a card because it is only asking for one
> device anyway. Using the card interface puts unnecessary overhead on both the
> driver and the pnp layer.

Yes, but IT SHOULD WORK. Although it isn't an most efficient way. (I
personally think that it's better to keep as much IDs as possible to avoid
clashes in future).

Jaroslav

-----
Jaroslav Kysela <[email protected]>
Linux Kernel Sound Maintainer
ALSA Project, SuSE Labs

2003-01-21 23:11:46

by Adam Belay

[permalink] [raw]
Subject: Re: [alsa, pnp] more on opl3sa2

On Tue, Jan 21, 2003 at 10:43:43PM +0100, Jaroslav Kysela wrote:
> On Tue, 21 Jan 2003, Adam Belay wrote:
> > 1.) If a driver attaches to the pnpbios card all other card-based drivers will
> > be unable to use the pnpbios. One will attach and cause the others to fail. It
> > is possible for the user to have more than one pnpbios sound card but with this
> > approach such a user would only be able to use one sound device from the entire
> > pnpbios.
>
> I see. I think it's a design problem then. The rule card -> one driver is
> bad. We need something between card and device which will take care about
> drivers. Unfortunately, this information is dynamic (only driver knows
> which devices have to be attached).
>
> I think that we need to discuss this thing very carefully.

I agree that we need to discuss this carefully and that there is a need for this
change. Here are a few ideas to get a discussion started.

How does this sound...
1.) detach pnp card service matching from the driver model, the driver model is
what is imposing this one card per driver limit.
2.) create a special pnp_driver that handles cards and forwards driver model calls
to the pnp card services, we can use attach_driver to avoid matching problems.

design goals for these changes should be as follows:
1.) multiple drivers can bind to one card
2.) pnp_attach, pnp_detach, and pnp status should be phased out and replaced with
the special card driver, in other words the driver model can take care of this.

This will solve the one device limit but I'm not sure how to handle the pnpbios
stuff yet. I want it to be with as little overhead as possible and would prefer we
don't use fake cards, any ideas? All that comes to mind for me is a unique pnpbios
ID that the pnp layer will interpret and grant special exceptions.

>
> > 2.) Doing so would misrepresent the pnpbios topology because it physically
> > doesn't have any cards.
> >
> > 3.) The opl3sa2 driver doesn't need a card because it is only asking for one
> > device anyway. Using the card interface puts unnecessary overhead on both the
> > driver and the pnp layer.
>
> Yes, but IT SHOULD WORK. Although it isn't an most efficient way. (I
> personally think that it's better to keep as much IDs as possible to avoid
> clashes in future).

Hmm, I'm not sure what you mean by clashes in the future. Nearly all of
these isapnp devices are no longer manufactured, hence these devices will
not have many future changes.

Still I do see value in describing as many ids as possible but I worry that the
extra overhead may not be worth it. This also needs to be further discussed.

Regards,
Adam

2003-01-22 02:29:00

by Kai Germaschewski

[permalink] [raw]
Subject: Re: [alsa, pnp] more on opl3sa2

On Tue, 21 Jan 2003, Adam Belay wrote:

> How does this sound...
> 1.) detach pnp card service matching from the driver model, the driver model is
> what is imposing this one card per driver limit.
> 2.) create a special pnp_driver that handles cards and forwards driver model calls
> to the pnp card services, we can use attach_driver to avoid matching problems.
>
> design goals for these changes should be as follows:
> 1.) multiple drivers can bind to one card
> 2.) pnp_attach, pnp_detach, and pnp status should be phased out and replaced with
> the special card driver, in other words the driver model can take care of this.

First of all I admit that I haven't been following closely, so I maybe way
off.

Anyway, the old ISAPnP used, AFAIR, struct pci_bus for the card and struct
pci_device for the devices. So what's wrong with using the basically the
same abstraction with the driver model, which has buses and devices as
well. That means each device can have its own driver, and I suppose that
should be good enough (as opposed to only one driver per card).

But probably I'm missing something?

--Kai


2003-01-22 08:14:34

by Jaroslav Kysela

[permalink] [raw]
Subject: Re: [alsa, pnp] more on opl3sa2

On Tue, 21 Jan 2003, Kai Germaschewski wrote:

> On Tue, 21 Jan 2003, Adam Belay wrote:
>
> > How does this sound...
> > 1.) detach pnp card service matching from the driver model, the driver model is
> > what is imposing this one card per driver limit.
> > 2.) create a special pnp_driver that handles cards and forwards driver model calls
> > to the pnp card services, we can use attach_driver to avoid matching problems.
> >
> > design goals for these changes should be as follows:
> > 1.) multiple drivers can bind to one card
> > 2.) pnp_attach, pnp_detach, and pnp status should be phased out and replaced with
> > the special card driver, in other words the driver model can take care of this.
>
> First of all I admit that I haven't been following closely, so I maybe way
> off.
>
> Anyway, the old ISAPnP used, AFAIR, struct pci_bus for the card and struct
> pci_device for the devices. So what's wrong with using the basically the
> same abstraction with the driver model, which has buses and devices as
> well. That means each device can have its own driver, and I suppose that
> should be good enough (as opposed to only one driver per card).
>
> But probably I'm missing something?

The structure is clear (devices & cards) but we need something like device
groups which contain subsets of devices per card. Actuall driver model
doesn't allow this directly, so we are looking for a way to implement
this.

Imagine:

card -+-> audio1
|
+-> audio2
|
+-> IDE

If we have two card drivers (one for audio1 & audio2 and second for IDE)
then current code will fail, because only one PnP driver can be attached
to a card.


Jaroslav

-----
Jaroslav Kysela <[email protected]>
Linux Kernel Sound Maintainer
ALSA Project, SuSE Labs

2003-01-22 09:35:14

by Jaroslav Kysela

[permalink] [raw]
Subject: Re: [alsa, pnp] more on opl3sa2

On Tue, 21 Jan 2003, Adam Belay wrote:

> On Tue, Jan 21, 2003 at 10:43:43PM +0100, Jaroslav Kysela wrote:
> > On Tue, 21 Jan 2003, Adam Belay wrote:
> > > 1.) If a driver attaches to the pnpbios card all other card-based drivers will
> > > be unable to use the pnpbios. One will attach and cause the others to fail. It
> > > is possible for the user to have more than one pnpbios sound card but with this
> > > approach such a user would only be able to use one sound device from the entire
> > > pnpbios.
> >
> > I see. I think it's a design problem then. The rule card -> one driver is
> > bad. We need something between card and device which will take care about
> > drivers. Unfortunately, this information is dynamic (only driver knows
> > which devices have to be attached).
> >
> > I think that we need to discuss this thing very carefully.
>
> I agree that we need to discuss this carefully and that there is a need for this
> change. Here are a few ideas to get a discussion started.
>
> How does this sound...
> 1.) detach pnp card service matching from the driver model, the driver model is
> what is imposing this one card per driver limit.
> 2.) create a special pnp_driver that handles cards and forwards driver model calls
> to the pnp card services, we can use attach_driver to avoid matching problems.
>
> design goals for these changes should be as follows:
> 1.) multiple drivers can bind to one card
> 2.) pnp_attach, pnp_detach, and pnp status should be phased out and replaced with
> the special card driver, in other words the driver model can take care of this.

Yes, it's quite clear except I think that the current pnp scheme is bad.
It would be probably better to have:

pnp_protocol -> pnp_card -> pnp_device

While pnp_protocol and pnp_card are buses (in the driver model) the
pnp_device is device. Actually, we have this model:

pnp_protocol (bus) -> pnp_card (device)
-> pnp_device (device)

Which don't really describe the real situation.

Also, we can revert to the previous probe() mechanism with device and card
IDs stored in one pnp_driver structure. We can export a function which
will chain the devices together (and pnp_device code will filter the
callbacks from the driver model to this chain of devices to remove
duplicate actions).

> This will solve the one device limit but I'm not sure how to handle the pnpbios
> stuff yet. I want it to be with as little overhead as possible and would prefer we
> don't use fake cards, any ideas? All that comes to mind for me is a unique pnpbios
> ID that the pnp layer will interpret and grant special exceptions.

It makes things a bit complicated. I personally like that all PnP BIOS
devices can enumerated separately via card_for_each_dev() and similar
macros without having any knowledge about used protocol. It's possible to
export the PnP BIOS card variable to make things more easy.

> > > 2.) Doing so would misrepresent the pnpbios topology because it physically
> > > doesn't have any cards.
> > >
> > > 3.) The opl3sa2 driver doesn't need a card because it is only asking for one
> > > device anyway. Using the card interface puts unnecessary overhead on both the
> > > driver and the pnp layer.
> >
> > Yes, but IT SHOULD WORK. Although it isn't an most efficient way. (I
> > personally think that it's better to keep as much IDs as possible to avoid
> > clashes in future).
>
> Hmm, I'm not sure what you mean by clashes in the future. Nearly all of
> these isapnp devices are no longer manufactured, hence these devices will
> not have many future changes.
>
> Still I do see value in describing as many ids as possible but I worry that the
> extra overhead may not be worth it. This also needs to be further discussed.

If you think in this way, then whole device model is overhead and we can
return to simple lists as before :-)

Jaroslav

-----
Jaroslav Kysela <[email protected]>
Linux Kernel Sound Maintainer
ALSA Project, SuSE Labs