2000-12-05 07:44:54

by Peter Samuelson

[permalink] [raw]
Subject: [PATCH] ymfpci.c MODULE_DEVICE_TABLE


Adam, could you check my work here? I haven't done this before.... It
compiles, but I don't have the hardware to verify anything. And, being
a lousy kernel hacker, I've probably introduced at least one bug.

Peter


diff -urk~ test12pre5/drivers/sound/Config.in~ test12pre5/drivers/sound/Config.in
--- test12pre5/drivers/sound/Config.in~ Mon Dec 4 23:59:44 2000
+++ test12pre5/drivers/sound/Config.in Tue Dec 5 01:02:05 2000
@@ -142,9 +142,9 @@
dep_tristate ' Yamaha FM synthesizer (YM3812/OPL-3) support' CONFIG_SOUND_YM3812 $CONFIG_SOUND_OSS
dep_tristate ' Yamaha OPL3-SA1 audio controller' CONFIG_SOUND_OPL3SA1 $CONFIG_SOUND_OSS
dep_tristate ' Yamaha OPL3-SA2, SA3, and SAx based PnP cards' CONFIG_SOUND_OPL3SA2 $CONFIG_SOUND_OSS
- dep_tristate ' Yamaha PCI legacy mode support' CONFIG_SOUND_YMPCI $CONFIG_SOUND_OSS $CONFIG_PCI
+ dep_tristate ' Yamaha YMF7xx PCI audio (legacy mode)' CONFIG_SOUND_YMPCI $CONFIG_SOUND_OSS $CONFIG_PCI
if [ "$CONFIG_SOUND_YMPCI" = "n" ]; then
- dep_tristate 'Yamaha PCI native mode support (EXPERIMENTAL)' CONFIG_SOUND_YMFPCI $CONFIG_SOUND_OSS
+ dep_tristate ' Yamaha YMF7xx PCI audio (native mode) (EXPERIMENTAL)' CONFIG_SOUND_YMFPCI $CONFIG_SOUND_OSS $CONFIG_PCI $CONFIG_EXPERIMENTAL
fi
dep_tristate ' 6850 UART support' CONFIG_SOUND_UART6850 $CONFIG_SOUND_OSS

diff -urk~ test12pre5/drivers/sound/ymfpci.c~ test12pre5/drivers/sound/ymfpci.c
--- test12pre5/drivers/sound/ymfpci.c~ Mon Dec 4 23:59:46 2000
+++ test12pre5/drivers/sound/ymfpci.c Tue Dec 5 00:59:44 2000
@@ -68,17 +68,19 @@
* constants
*/

-static struct ymf_devid {
- int id;
- char *name;
-} ymf_devv[] = {
- { PCI_DEVICE_ID_YAMAHA_724, "YMF724" },
- { PCI_DEVICE_ID_YAMAHA_724F, "YMF724F" },
- { PCI_DEVICE_ID_YAMAHA_740, "YMF740" },
- { PCI_DEVICE_ID_YAMAHA_740C, "YMF740C" },
- { PCI_DEVICE_ID_YAMAHA_744, "YMF744" },
- { PCI_DEVICE_ID_YAMAHA_754, "YMF754" },
+static struct pci_device_id ymf_id_tbl[] = {
+#define DEV(v, d, data) \
+ { PCI_VENDOR_ID_##v, PCI_DEVICE_ID_##v##_##d, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (unsigned long)data }
+ DEV (YAMAHA, 724, "YMF724"),
+ DEV (YAMAHA, 724F, "YMF724F"),
+ DEV (YAMAHA, 740, "YMF740"),
+ DEV (YAMAHA, 740C, "YMF740C"),
+ DEV (YAMAHA, 744, "YMF744"),
+ DEV (YAMAHA, 754, "YMF754"),
+#undef DEV
+ { }
};
+MODULE_DEVICE_TABLE(pci, ymf_id_tbl);

/*
* Mindlessly copied from cs46xx XXX
@@ -2245,8 +2247,10 @@
return 0;
}

-static int /* __init */
-ymf_install(struct pci_dev *pcidev, int instance, int devx)
+static int ymf_instance;
+
+static int __devinit
+ymf_install_one(struct pci_dev *pcidev, const struct pci_device_id *ent)
{
ymfpci_t *codec;

@@ -2261,7 +2265,7 @@
spin_lock_init(&codec->reg_lock);
spin_lock_init(&codec->voice_lock);
codec->pci = pcidev;
- codec->inst = instance;
+ codec->inst = ymf_instance;
codec->irq = pcidev->irq;
codec->device_id = pcidev->device;
pci_read_config_byte(pcidev, PCI_REVISION_ID, (u8 *)&codec->rev);
@@ -2270,8 +2274,8 @@
pci_set_master(pcidev);

/* XXX KERN_INFO */
- printk("ymfpci%d: %s at 0x%lx IRQ %d\n", instance,
- ymf_devv[devx].name, codec->reg_area_phys, codec->irq);
+ printk("ymfpci%d: %s at 0x%lx IRQ %d\n", ymf_instance,
+ (char *)ent->driver_data, codec->reg_area_phys, codec->irq);

ymfpci_aclink_reset(pcidev);
if (ymfpci_codec_ready(codec, 0, 1) < 0) {
@@ -2317,6 +2321,7 @@

codec->next = ymf_devs;
ymf_devs = codec;
+ ymf_instance++;

return 0;
}
@@ -2356,43 +2361,15 @@
MODULE_AUTHOR("Jaroslav Kysela");
MODULE_DESCRIPTION("Yamaha YMF7xx PCI Audio");

-static int /* __init */
-ymf_probe(void)
-{
- struct pci_dev *pcidev;
- int foundone;
- int i;
-
- if (!pci_present())
- return -ENODEV;
-
- /*
- * Print our proud ego-nursing "Hello, World!" message as in MS-DOS.
- */
- /* printk(KERN_INFO "ymfpci: Yamaha YMF7xx\n"); */
-
- /*
- * Not very efficient, but Alan did it so in cs46xx.c.
- */
- foundone = 0;
- pcidev = NULL;
- for (i = 0; i < sizeof(ymf_devv)/sizeof(ymf_devv[0]); i++) {
- while ((pcidev = pci_find_device(PCI_VENDOR_ID_YAMAHA,
- ymf_devv[i].id, pcidev)) != NULL) {
- if (ymf_install(pcidev, foundone, i) == 0) {
- foundone++;
- }
- }
- }
-
- return foundone;
-}
+static struct pci_driver ymfpci_driver = {
+ name: "ymfpci",
+ id_table: ymf_id_tbl,
+ probe: ymf_install_one,
+};

static int ymf_init_module(void)
{
- if (ymf_probe()==0)
- printk(KERN_ERR "ymfpci: No devices found.\n");
- return 0;
+ return pci_module_init (&ymfpci_driver);
}

static void ymf_cleanup_module (void)


2000-12-05 12:42:08

by Kai Germaschewski

[permalink] [raw]
Subject: Re: [PATCH] ymfpci.c MODULE_DEVICE_TABLE



On Tue, 5 Dec 2000, Peter Samuelson wrote:

> Adam, could you check my work here? I haven't done this before.... It
> compiles, but I don't have the hardware to verify anything. And, being
> a lousy kernel hacker, I've probably introduced at least one bug.

The driver works here. I had forward ported ymfpci myself before, and I
didn't work. Now Linus' version didn't work either, so I decided to
investigate a little further.

For 2.2 I need to disable PnP OS in the BIOS, for 2.4 I had it enabled.
That apparently made the difference, the driver works only when PnP OS is
disabled (2.2 and 2.4).

Your patch works fine here, didn't make a difference, though (of course).

I noticed that pci_enable_device isn't called, so I added the following
(proposed for inclusion)

--- linux-2.4.0-test12-pre5.compile/drivers/sound/ymfpci.c~ Tue Dec 5 11:18:50 2000
+++ linux-2.4.0-test12-pre5.compile/drivers/sound/ymfpci.c Tue Dec 5 11:21:38 2000
@@ -2256,6 +2256,10 @@

int err;

+ if (pci_enable_device(pcidev) < 0) {
+ printk(KERN_ERR "ymfpci: pci_enable_device failed\n");
+ return -ENODEV;
+ }
if ((codec = kmalloc(sizeof(ymfpci_t), GFP_KERNEL)) == NULL) {
printk(KERN_ERR "ymfpci: no core\n");
return -ENOMEM;

However, that didn't help either.
Driver load messages:

PCI: Enabling device 00:09.0 (0006 -> 0007)
ymfpci0: YMF744 at 0xfecf0000 IRQ 9
ac97_codec: AC97 Audio codec, id: 0x414b:0x4d02 (Unknown)

The IO regions really get enabled:

(diff lspci -vx before / after loading)

--- /root/lspci.0 Tue Dec 5 12:30:19 2000
+++ /root/lspci.1 Tue Dec 5 12:32:03 2000
@@ -63,12 +63,12 @@
00:09.0 Multimedia audio controller: Yamaha Corporation YMF-744B [DS-1S Audio Controller] (rev 02)
Subsystem: Sony Corporation: Unknown device 8081
Flags: bus master, medium devsel, latency 64, IRQ 9
Memory at fecf0000 (32-bit, non-prefetchable) [size=32K]
- I/O ports at fc00 [disabled] [size=64]
- I/O ports at fcac [disabled] [size=4]
+ I/O ports at fc00 [size=64]
+ I/O ports at fcac [size=4]
Capabilities: [50] Power Management version 1
-00: 73 10 10 00 06 00 10 02 02 00 01 04 00 40 00 00
+00: 73 10 10 00 07 00 10 02 02 00 01 04 00 40 00 00
10: 00 00 cf fe 01 fc 00 00 ad fc 00 00 00 00 00 00
20: 00 00 00 00 00 00 00 00 00 00 00 00 4d 10 81 80
30: 00 00 00 00 50 00 00 00 00 00 00 00 09 01 05 19

but it doesn't really help, sound just doesn't work properly (it starts
out okay, but then it starts skipping after a second or so and eventually
it totally stops).

For comparison, lspci -vx output when booted with Pnp OS = no (-> sound
works properly):

00:09.0 Multimedia audio controller: Yamaha Corporation YMF-744B [DS-1S Audio Controller] (rev 02)
Subsystem: Sony Corporation: Unknown device 8081
Flags: bus master, medium devsel, latency 64, IRQ 9
Memory at fedf8000 (32-bit, non-prefetchable) [size=32K]
I/O ports at fcc0 [size=64]
I/O ports at fc8c [size=4]
Capabilities: [50] Power Management version 1
00: 73 10 10 00 07 00 10 02 02 00 01 04 00 40 00 00
10: 00 80 df fe c1 fc 00 00 8d fc 00 00 00 00 00 00
20: 00 00 00 00 00 00 00 00 00 00 00 00 4d 10 81 80
30: 00 00 00 00 50 00 00 00 00 00 00 00 09 01 05 19

Any idea why this setup works but the previous one doesn't? But maybe the
BIOS initializes the sound chip differently depending on the selection of
PnP OS...

BTW, this is also a Sony notebook (PCG-Z600NE), not a Transmeta one,
though :-(

Talking about it, there's also yet another problem with this notebook. The
A20 patch by hpa in test12-pre broke resume from suspend/hibernation - the
notebook just silently reboots after a resume - any idea why that would
be? (The back-out patch I'm using is available on request)

--Kai