mpu401_pnp

mpu401_pnp

[castet.matthieu]

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


Hello,

I send you a mpu401 modules that support pnpbios.
I don't add it to the mpu401 because it would add too much define.
I am not a specialist in Kconfig edit, so for testing on my comptuter, I only
remplace traditionnal mpu401 with mpu401_pnp.

Also I clear the function that create the mpu (snd_card_mpu401_add), and I think
you could re-use it for acpi_pnp insted of adding define.

As you can see, because of a bug in pnpbios code, sometime it doesn't find a
irq, so it add a litle hack waiting the bug is fix in the kernel.

With that, an some script (hotplug for example), the mpu401 is automatiquely
detected(at least on my i810 chipset).

Matthieu

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: mpu401_pnp.c --]
[-- Type: text/x-csrc; name="mpu401_pnp.c", Size: 5575 bytes --]

/*
 *  Driver for generic MPU-401 boards (UART mode only)
 *  Copyright (c) by Jaroslav Kysela <perex@suse.cz>
 *
 *  Copyright (c) 2004 by Castet Matthieu <castet.matthieu@free.fr>
 *  portd on mpu401.c
 *  Copyright (c) 2002-2003 Matthew Wilcox for Hewlett-Packard
 *  Copyright (C) 2004 Hewlett-Packard Co
 *       Bjorn Helgaas <bjorn.helgaas@hp.com>
 *
 *
 *   This program is free software; you can redistribute it and/or modify
 *   it under the terms of the GNU General Public License as published by
 *   the Free Software Foundation; either version 2 of the License, or
 *   (at your option) any later version.
 *
 *   This program is distributed in the hope that it will be useful,
 *   but WITHOUT ANY WARRANTY; without even the implied warranty of
 *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 *   GNU General Public License for more details.
 *
 *   You should have received a copy of the GNU General Public License
 *   along with this program; if not, write to the Free Software
 *   Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 USA
 *
 */

#include <sound/driver.h>
#include <linux/init.h>
#include <linux/moduleparam.h>
#include <sound/core.h>
#include <sound/mpu401.h>
#include <sound/initval.h>
#include <linux/pnp.h>


MODULE_AUTHOR("Jaroslav Kysela <perex@suse.cz>");
MODULE_DESCRIPTION("MPU-401 UART");
MODULE_LICENSE("GPL");
MODULE_CLASSES("{sound}");

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 int boot_devs;

module_param_array(index, int, boot_devs, 0444);
MODULE_PARM_DESC(index, "Index value for MPU-401 device.");
MODULE_PARM_SYNTAX(index, SNDRV_INDEX_DESC);
module_param_array(id, charp, boot_devs, 0444);
MODULE_PARM_DESC(id, "ID string for MPU-401 device.");
MODULE_PARM_SYNTAX(id, SNDRV_ID_DESC);
module_param_array(enable, bool, boot_devs, 0444);
MODULE_PARM_DESC(enable, "Enable MPU-401 device.");
MODULE_PARM_SYNTAX(enable, SNDRV_ENABLE_DESC);

#define IO_EXTENT 2

#define BROKEN_PNPBIOS

static int snd_card_mpu401_add(int dev, snd_card_t **pcard, long port, int irq, char * optional_description) 
{
	int err;
	snd_card_t *card;

	card = snd_card_new(index[dev], id[dev], THIS_MODULE, 0);
	if (card == NULL)
		return -ENOMEM;
	strcpy(card->driver, "MPU-401 UART");
	strcpy(card->shortname, card->driver);
	sprintf(card->longname, "%s at 0x%lx, ", card->shortname, port);
	if (irq >= 0) {
		sprintf(card->longname + strlen(card->longname), "IRQ %d", irq);
	} else {
		strcat(card->longname, "polled");
	}
	if (optional_description)
		strlcat(card->longname, optional_description, sizeof(card->longname));

	if (snd_mpu401_uart_new(card, 0,
				MPU401_HW_MPU401,
				port, 0,
				irq, irq >= 0 ? SA_INTERRUPT : 0, NULL) < 0) {
		printk(KERN_ERR "MPU401 not detected at 0x%lx\n", port);
		snd_card_free(card);
		return -ENODEV;
	}
	if ((err = snd_card_register(card)) < 0) {
		snd_card_free(card);
		return err;
	}
	*pcard = card;
	return 0;
}

static struct pnp_device_id pnp_devids[] = {
	{ .id = "PNPb006", .driver_data = 0 },
	{ .id = "", },
};

MODULE_DEVICE_TABLE(pnp, pnp_devids);

static int __devinit snd_card_mpu401_pnp_probe(struct pnp_dev *dev, const struct pnp_device_id *did) {
	long port;
	int irq;
	snd_card_t *card = NULL;
	int ret;
	
	static int card_dev = 0;

	if (card_dev >= SNDRV_CARDS)
		return -ENODEV;

	if (!enable[card_dev]) {
		card_dev++;
		return -ENOENT;
	}

#ifdef BROKEN_PNPBIOS
retry:
#endif
	if (!pnp_port_valid(dev, 0) ||
			pnp_port_flags(dev,0) & IORESOURCE_DISABLED) {
		printk(KERN_WARNING "No io port !\n");
		return -ENODEV;
	}
	if (pnp_port_len(dev, 0) < IO_EXTENT) {
		printk(KERN_WARNING "io port len : %ld (expected : %d) !\n",
				pnp_port_len(dev, 0), 
				IO_EXTENT);
		return -ENODEV;
	}
	port = pnp_port_start(dev,0);

	if (!pnp_irq_valid(dev,0)||
			pnp_irq_flags(dev,0) & IORESOURCE_DISABLED) {
#ifdef BROKEN_PNPBIOS
		//XXX parser bug
		static int retry = 0;
		if (dev->dependent && !retry) {
			dev->active = 0;
			printk(KERN_WARNING "broken pnpbios retry : %d!\n",pnp_assign_resources(dev, 2));
			dev->active = 1;
			retry = 1;
			goto retry;
		}
#endif
		printk(KERN_WARNING "No irq !\n");
		irq = -1;
	}
	else {
		irq = pnp_irq(dev,0);
	}

	printk(KERN_INFO "snd_card_mpu401 pnpbios :"
		"%s port 0x%03lx, irq=%d\n",
		pnp_dev_name(dev), port, irq);
	
	//XXX pnp_dev_name(pnp_dev) give me a empty string ...
	/*
	strcat(option, ", ");
	strlcat(option, pnp_dev_name(pnp_dev), sizeof(card->longname));
	*/
	ret = snd_card_mpu401_add(card_dev, &card, port, irq, NULL); 
	if (ret >= 0)
		pnp_set_drvdata(dev, card);

	return ret;
}

static void snd_card_mpu401_pnp_remove(struct pnp_dev *dev) {
	snd_card_t *card = (snd_card_t *)pnp_get_drvdata(dev);
	if (!card) {
		BUG();
		return;
	}

	snd_card_disconnect(card);
	snd_card_free_in_thread(card);
}
	
static struct pnp_driver snd_mpu401_pnp_driver = {
	.name           = "mpu401",
	.id_table       = pnp_devids,
	.probe          = snd_card_mpu401_pnp_probe,
	.remove		= snd_card_mpu401_pnp_remove,
};

static int __init alsa_card_mpu401_init(void)
{
	int ret = pnp_register_driver(&snd_mpu401_pnp_driver);
	if (ret == 0) {
		pnp_unregister_driver(&snd_mpu401_pnp_driver);
	}
		
	if (ret <= 0) {
		return -ENODEV;
	}

	return 0;
}

static void __exit alsa_card_mpu401_exit(void)
{
	pnp_unregister_driver(&snd_mpu401_pnp_driver);
}

module_init(alsa_card_mpu401_init)
module_exit(alsa_card_mpu401_exit)

Re: mpu401_pnp

[Clemens Ladisch]

castet.matthieu@free.fr wrote:
> I send you a mpu401 modules that support pnpbios.

Applied (merged with mpu401.c), thanks.

(I didn't read your mail from Friday until I had written the patch. :)

> As you can see, because of a bug in pnpbios code, sometime it doesn't find a
> irq, so it add a litle hack waiting the bug is fix in the kernel.

I didn't add this workaround.  Is this bug still in the kernel?


Best regards,
Clemens



-------------------------------------------------------
This SF.Net email is sponsored by: IntelliVIEW -- Interactive Reporting
Tool for open source databases. Create drag-&-drop reports. Save time
by over 75%! Publish reports on the web. Export to DOC, XLS, RTF, etc.
Download a FREE copy at http://www.intelliview.com/go/osdn_nl

Re: mpu401_pnp

[castet.matthieu]

Quoting Clemens Ladisch <clemens@ladisch.de>:

> castet.matthieu@free.fr wrote:
> > I send you a mpu401 modules that support pnpbios.
>
> Applied (merged with mpu401.c), thanks.
>
> (I didn't read your mail from Friday until I had written the patch. :)
>
> > As you can see, because of a bug in pnpbios code, sometime it doesn't find
> a
> > irq, so it add a litle hack waiting the bug is fix in the kernel.
>
> I didn't add this workaround.  Is this bug still in the kernel?
>
No it was corrected in 2.6.9 or 2.6.10.

Regards,

Matthieu




-------------------------------------------------------
This SF.Net email is sponsored by: IntelliVIEW -- Interactive Reporting
Tool for open source databases. Create drag-&-drop reports. Save time
by over 75%! Publish reports on the web. Export to DOC, XLS, RTF, etc.
Download a FREE copy at http://www.intelliview.com/go/osdn_nl

Re: mpu401_pnp

[matthieu castet]

Hi,
Clemens Ladisch wrote:
> castet.matthieu@free.fr wrote:
> 
>>I send you a mpu401 modules that support pnpbios.
> 
> 
> Applied (merged with mpu401.c), thanks.
> 
> (I didn't read your mail from Friday until I had written the patch. :)
Ok, I found a bug in your patch : when you call pnp_register_driver, you 
should check that it is >= 0. If not you shouldn't call 
pnp_unregister_driver.


Matthieu


-------------------------------------------------------
This SF.Net email is sponsored by: IntelliVIEW -- Interactive Reporting
Tool for open source databases. Create drag-&-drop reports. Save time
by over 75%! Publish reports on the web. Export to DOC, XLS, RTF, etc.
Download a FREE copy at http://www.intelliview.com/go/osdn_nl

Re: mpu401_pnp

[matthieu castet]

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

Hi,

attached a version more readable with less ifdef.
It is untested, but it should work.

One question : does snd_card_set_dev(card, &device->dev) is important ? 
It wasn't use with acpi.

regards,

Matthieu

[-- Attachment #2: mpu.patch --]
[-- Type: text/x-patch, Size: 5318 bytes --]

--- mpu401.c?rev=1.22	2005-02-07 13:26:09.000000000 +0100
+++ mpu401.c	2005-02-07 13:55:30.000000000 +0100
@@ -57,68 +57,13 @@
 MODULE_PARM_DESC(irq, "IRQ # for MPU-401 device.");
 
 static snd_card_t *snd_mpu401_legacy_cards[SNDRV_CARDS] = SNDRV_DEFAULT_PTR;
+static int pnp_registered = 0;
 
-#ifdef CONFIG_PNP
-
-#define IO_EXTENT 2
-
-static struct pnp_device_id snd_mpu401_pnpids[] = {
-	{ .id = "PNPb006" },
-	{ .id = "" }
-};
-
-MODULE_DEVICE_TABLE(pnp, snd_mpu401_pnpids);
-
-static int __init snd_mpu401_pnp(int dev, struct pnp_dev *device,
-				 const struct pnp_device_id *id)
-{
-	if (!pnp_port_valid(device, 0) ||
-	    pnp_port_flags(device, 0) & IORESOURCE_DISABLED) {
-		snd_printk(KERN_ERR "no PnP port\n");
-		return -ENODEV;
-	}
-	if (pnp_port_len(device, 0) < IO_EXTENT) {
-		snd_printk(KERN_ERR "PnP port length is %ld, expected %d\n",
-			   pnp_port_len(device, 0), IO_EXTENT);
-		return -ENODEV;
-	}
-	port[dev] = pnp_port_start(device, 0);
-
-	if (!pnp_irq_valid(device, 0) ||
-	    pnp_irq_flags(device, 0) & IORESOURCE_DISABLED) {
-		snd_printk(KERN_WARNING "no PnP irq, using polling\n");
-		irq[dev] = -1;
-	} else {
-		irq[dev] = pnp_irq(device, 0);
-	}
-	return 0;
-}
-#endif
-
-static int __devinit snd_card_mpu401_probe(int dev, struct pnp_dev *device,
-					   const struct pnp_device_id *pnp_id)
+static int snd_card_mpu401_add(int dev)
 {
 	snd_card_t *card;
 	int err;
 
-#ifdef CONFIG_PNP
-	if (!device) {
-#endif
-		if (port[dev] == SNDRV_AUTO_PORT) {
-			snd_printk(KERN_ERR "specify port\n");
-			return -EINVAL;
-		}
-		if (irq[dev] == SNDRV_AUTO_IRQ) {
-			snd_printk(KERN_ERR "specify or disable IRQ\n");
-			return -EINVAL;
-		}
-#ifdef CONFIG_PNP
-	} else {
-		if ((err = snd_mpu401_pnp(dev, device, pnp_id)) < 0)
-			return err;
-	}
-#endif
-
 	card = snd_card_new(index[dev], id[dev], THIS_MODULE, 0);
 	if (card == NULL)
 		return -ENOMEM;
@@ -130,11 +75,13 @@
 	} else {
 		strcat(card->longname, "polled");
 	}
-#ifdef CONFIG_PNP
+
+	/*
 	if (device) {
 		snd_card_set_dev(card, &device->dev);
 	}
-#endif
+	*/
+
 	if (snd_mpu401_uart_new(card, 0,
 				MPU401_HW_MPU401,
 				port[dev], 0,
@@ -147,16 +94,59 @@
 		snd_card_free(card);
 		return err;
 	}
-#ifdef CONFIG_PNP
-	if (device)
-		pnp_set_drvdata(device, card);
-	else
-#endif
 		snd_mpu401_legacy_cards[dev] = card;
 	return 0;
 }
 
+static int __devinit snd_card_mpu401_probe(int dev)
+{
+	if (port[dev] == SNDRV_AUTO_PORT) {
+		snd_printk(KERN_ERR "specify port\n");
+		return -EINVAL;
+	}
+	if (irq[dev] == SNDRV_AUTO_IRQ) {
+		snd_printk(KERN_ERR "specify or disable IRQ\n");
+		return -EINVAL;
+	}
+	return snd_card_mpu401_add(dev);
+}
+
 #ifdef CONFIG_PNP
+
+#define IO_EXTENT 2
+
+static struct pnp_device_id snd_mpu401_pnpids[] = {
+	{ .id = "PNPb006" },
+	{ .id = "" }
+};
+
+MODULE_DEVICE_TABLE(pnp, snd_mpu401_pnpids);
+
+static int __init snd_mpu401_pnp(int dev, struct pnp_dev *device,
+				 const struct pnp_device_id *id)
+{
+	if (!pnp_port_valid(device, 0) ||
+	    pnp_port_flags(device, 0) & IORESOURCE_DISABLED) {
+		snd_printk(KERN_ERR "no PnP port\n");
+		return -ENODEV;
+	}
+	if (pnp_port_len(device, 0) < IO_EXTENT) {
+		snd_printk(KERN_ERR "PnP port length is %ld, expected %d\n",
+			   pnp_port_len(device, 0), IO_EXTENT);
+		return -ENODEV;
+	}
+	port[dev] = pnp_port_start(device, 0);
+
+	if (!pnp_irq_valid(device, 0) ||
+	    pnp_irq_flags(device, 0) & IORESOURCE_DISABLED) {
+		snd_printk(KERN_WARNING "no PnP irq, using polling\n");
+		irq[dev] = -1;
+	} else {
+		irq[dev] = pnp_irq(device, 0);
+	}
+	return snd_card_mpu401_add(dev);
+}
+
 static int __devinit snd_mpu401_pnp_probe(struct pnp_dev *pnp_dev,
 					  const struct pnp_device_id *id)
 {
@@ -166,9 +156,11 @@
 	for ( ; dev < SNDRV_CARDS; ++dev) {
 		if (!enable[dev] || !pnp[dev])
 			continue;
-		err = snd_card_mpu401_probe(dev, pnp_dev, id);
+		err = snd_mpu401_pnp(dev, pnp_dev, id);
 		if (err < 0)
 			return err;
+		pnp_set_drvdata(pnp_dev, snd_mpu401_legacy_cards[dev]);
+		snd_mpu401_legacy_cards[dev] = NULL;
 		++dev;
 		return 0;
 	}
@@ -189,11 +181,14 @@
 	.probe = snd_mpu401_pnp_probe,
 	.remove = __devexit_p(snd_mpu401_pnp_remove),
 };
+#else
+static struct pnp_driver snd_mpu401_pnp_driver;
 #endif
 
 static int __init alsa_card_mpu401_init(void)
 {
 	int dev, devices = 0;
+	int err;
 
 	for (dev = 0; dev < SNDRV_CARDS; dev++) {
 		if (!enable[dev])
@@ -202,19 +197,20 @@
 		if (pnp[dev])
 			continue;
 #endif
-		if (snd_card_mpu401_probe(dev, NULL, NULL) >= 0)
+		if (snd_card_mpu401_probe(dev) >= 0)
 			devices++;
 	}
-#ifdef CONFIG_PNP
-	devices += pnp_register_driver(&snd_mpu401_pnp_driver);
-#endif
+	if ((err = pnp_register_driver(&snd_mpu401_pnp_driver)) >= 0) {
+		pnp_registered = 1;
+		devices += err;
+	}
+
 	if (!devices) {
 #ifdef MODULE
 		printk(KERN_ERR "MPU-401 device not found or device busy\n");
 #endif
-#ifdef CONFIG_PNP
-		pnp_unregister_driver(&snd_mpu401_pnp_driver);
-#endif
+		if (pnp_registered)
+			pnp_unregister_driver(&snd_mpu401_pnp_driver);
 		return -ENODEV;
 	}
 	return 0;
@@ -224,9 +220,8 @@
 {
 	int idx;
 
-#ifdef CONFIG_PNP
-	pnp_unregister_driver(&snd_mpu401_pnp_driver);
-#endif
+	if (pnp_registered)
+		pnp_unregister_driver(&snd_mpu401_pnp_driver);
 	for (idx = 0; idx < SNDRV_CARDS; idx++)
 		snd_card_free(snd_mpu401_legacy_cards[idx]);
 }

Re: mpu401_pnp

[Clemens Ladisch]

matthieu castet wrote:
> Ok, I found a bug in your patch : when you call pnp_register_driver, you
> should check that it is >= 0. If not you shouldn't call
> pnp_unregister_driver.

I just copied this from the other ALSA PnP drivers.  It seems those
have all the same bug.

> One question : does snd_card_set_dev(card, &device->dev) is important ?

Well, this pointer should be set if possible, but I'm not sure if it's
actually used anywhere.

> It wasn't use with acpi.

Well, it should have been used. :-)  I adapted the ACPI code from the
serial driver which of course didn't have this, and the 2.4 kernel I
developed this on didn't have 'struct device' anyway.


Best regards,
Clemens



-------------------------------------------------------
This SF.Net email is sponsored by: IntelliVIEW -- Interactive Reporting
Tool for open source databases. Create drag-&-drop reports. Save time
by over 75%! Publish reports on the web. Export to DOC, XLS, RTF, etc.
Download a FREE copy at http://www.intelliview.com/go/osdn_nl

Re: mpu401_pnp

[matthieu castet]

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

Hi,

Clemens Ladisch wrote:
>>One question : does snd_card_set_dev(card, &device->dev) is important ?
> 
> 
> Well, this pointer should be set if possible, but I'm not sure if it's
> actually used anywhere.
> 
Ok, I update my patch in order to use it.

Regards

Matthieu

[-- Attachment #2: mpu.patch --]
[-- Type: text/x-patch, Size: 4950 bytes --]

--- mpu401.c?rev=1.22	2005-02-07 20:08:05.000000000 +0100
+++ mpu401.c	2005-02-07 20:07:57.000000000 +0100
@@ -57,6 +57,53 @@
 MODULE_PARM_DESC(irq, "IRQ # for MPU-401 device.");
 
 static snd_card_t *snd_mpu401_legacy_cards[SNDRV_CARDS] = SNDRV_DEFAULT_PTR;
+static int pnp_registered = 0;
+
+static int snd_card_mpu401_add(int dev)
+{
+	snd_card_t *card;
+	int err;
+
+	card = snd_card_new(index[dev], id[dev], THIS_MODULE, 0);
+	if (card == NULL)
+		return -ENOMEM;
+	strcpy(card->driver, "MPU-401 UART");
+	strcpy(card->shortname, card->driver);
+	sprintf(card->longname, "%s at %#lx, ", card->shortname, port[dev]);
+	if (irq[dev] >= 0) {
+		sprintf(card->longname + strlen(card->longname), "irq %d", irq[dev]);
+	} else {
+		strcat(card->longname, "polled");
+	}
+
+	if (snd_mpu401_uart_new(card, 0,
+				MPU401_HW_MPU401,
+				port[dev], 0,
+				irq[dev], irq[dev] >= 0 ? SA_INTERRUPT : 0, NULL) < 0) {
+		printk(KERN_ERR "MPU401 not detected at 0x%lx\n", port[dev]);
+		snd_card_free(card);
+		return -ENODEV;
+	}
+	if ((err = snd_card_register(card)) < 0) {
+		snd_card_free(card);
+		return err;
+	}
+		snd_mpu401_legacy_cards[dev] = card;
+	return 0;
+}
+
+static int __devinit snd_card_mpu401_probe(int dev)
+{
+	if (port[dev] == SNDRV_AUTO_PORT) {
+		snd_printk(KERN_ERR "specify port\n");
+		return -EINVAL;
+	}
+	if (irq[dev] == SNDRV_AUTO_IRQ) {
+		snd_printk(KERN_ERR "specify or disable IRQ\n");
+		return -EINVAL;
+	}
+	return snd_card_mpu401_add(dev);
+}
 
 #ifdef CONFIG_PNP
 
@@ -91,72 +138,9 @@
 	} else {
 		irq[dev] = pnp_irq(device, 0);
 	}
-	return 0;
+	return snd_card_mpu401_add(dev);
 }
-#endif
-
-static int __devinit snd_card_mpu401_probe(int dev, struct pnp_dev *device,
-					   const struct pnp_device_id *pnp_id)
-{
-	snd_card_t *card;
-	int err;
 
-#ifdef CONFIG_PNP
-	if (!device) {
-#endif
-		if (port[dev] == SNDRV_AUTO_PORT) {
-			snd_printk(KERN_ERR "specify port\n");
-			return -EINVAL;
-		}
-		if (irq[dev] == SNDRV_AUTO_IRQ) {
-			snd_printk(KERN_ERR "specify or disable IRQ\n");
-			return -EINVAL;
-		}
-#ifdef CONFIG_PNP
-	} else {
-		if ((err = snd_mpu401_pnp(dev, device, pnp_id)) < 0)
-			return err;
-	}
-#endif
-
-	card = snd_card_new(index[dev], id[dev], THIS_MODULE, 0);
-	if (card == NULL)
-		return -ENOMEM;
-	strcpy(card->driver, "MPU-401 UART");
-	strcpy(card->shortname, card->driver);
-	sprintf(card->longname, "%s at %#lx, ", card->shortname, port[dev]);
-	if (irq[dev] >= 0) {
-		sprintf(card->longname + strlen(card->longname), "irq %d", irq[dev]);
-	} else {
-		strcat(card->longname, "polled");
-	}
-#ifdef CONFIG_PNP
-	if (device) {
-		snd_card_set_dev(card, &device->dev);
-	}
-#endif
-	if (snd_mpu401_uart_new(card, 0,
-				MPU401_HW_MPU401,
-				port[dev], 0,
-				irq[dev], irq[dev] >= 0 ? SA_INTERRUPT : 0, NULL) < 0) {
-		printk(KERN_ERR "MPU401 not detected at 0x%lx\n", port[dev]);
-		snd_card_free(card);
-		return -ENODEV;
-	}
-	if ((err = snd_card_register(card)) < 0) {
-		snd_card_free(card);
-		return err;
-	}
-#ifdef CONFIG_PNP
-	if (device)
-		pnp_set_drvdata(device, card);
-	else
-#endif
-		snd_mpu401_legacy_cards[dev] = card;
-	return 0;
-}
-
-#ifdef CONFIG_PNP
 static int __devinit snd_mpu401_pnp_probe(struct pnp_dev *pnp_dev,
 					  const struct pnp_device_id *id)
 {
@@ -166,9 +150,12 @@
 	for ( ; dev < SNDRV_CARDS; ++dev) {
 		if (!enable[dev] || !pnp[dev])
 			continue;
-		err = snd_card_mpu401_probe(dev, pnp_dev, id);
+		err = snd_mpu401_pnp(dev, pnp_dev, id);
 		if (err < 0)
 			return err;
+		snd_card_set_dev(snd_mpu401_legacy_cards[dev], &device->dev);
+		pnp_set_drvdata(pnp_dev, snd_mpu401_legacy_cards[dev]);
+		snd_mpu401_legacy_cards[dev] = NULL;
 		++dev;
 		return 0;
 	}
@@ -189,11 +176,14 @@
 	.probe = snd_mpu401_pnp_probe,
 	.remove = __devexit_p(snd_mpu401_pnp_remove),
 };
+#else
+static struct pnp_driver snd_mpu401_pnp_driver;
 #endif
 
 static int __init alsa_card_mpu401_init(void)
 {
 	int dev, devices = 0;
+	int err;
 
 	for (dev = 0; dev < SNDRV_CARDS; dev++) {
 		if (!enable[dev])
@@ -202,19 +192,20 @@
 		if (pnp[dev])
 			continue;
 #endif
-		if (snd_card_mpu401_probe(dev, NULL, NULL) >= 0)
+		if (snd_card_mpu401_probe(dev) >= 0)
 			devices++;
 	}
-#ifdef CONFIG_PNP
-	devices += pnp_register_driver(&snd_mpu401_pnp_driver);
-#endif
+	if ((err = pnp_register_driver(&snd_mpu401_pnp_driver)) >= 0) {
+		pnp_registered = 1;
+		devices += err;
+	}
+
 	if (!devices) {
 #ifdef MODULE
 		printk(KERN_ERR "MPU-401 device not found or device busy\n");
 #endif
-#ifdef CONFIG_PNP
-		pnp_unregister_driver(&snd_mpu401_pnp_driver);
-#endif
+		if (pnp_registered)
+			pnp_unregister_driver(&snd_mpu401_pnp_driver);
 		return -ENODEV;
 	}
 	return 0;
@@ -224,9 +215,8 @@
 {
 	int idx;
 
-#ifdef CONFIG_PNP
-	pnp_unregister_driver(&snd_mpu401_pnp_driver);
-#endif
+	if (pnp_registered)
+		pnp_unregister_driver(&snd_mpu401_pnp_driver);
 	for (idx = 0; idx < SNDRV_CARDS; idx++)
 		snd_card_free(snd_mpu401_legacy_cards[idx]);
 }

Re: mpu401_pnp

[matthieu castet]

Hi,
Clemens Ladisch wrote:
> matthieu castet wrote:
> 
>>Ok, I found a bug in your patch : when you call pnp_register_driver, you
>>should check that it is >= 0. If not you shouldn't call
>>pnp_unregister_driver.
> 
> 
> I just copied this from the other ALSA PnP drivers.  It seems those
> have all the same bug.
$grep -rI pnp_register_driver /usr/src/linux/sound/
/usr/src/linux/sound/oss/opl3sa2.c: 
pnp_register_driver(&opl3sa2_driver);
/usr/src/linux/sound/oss/awe_wave.c:    ret = 
pnp_register_driver(&awe_pnp_driver);
/usr/src/linux/sound/oss/cs4232.c: 
(pnp_register_driver(&cs4232_driver) > 0)
/usr/src/linux/sound/oss/cs4232.c:      if 
(pnp_register_driver(&cs4232_driver) > 0)
/usr/src/linux/sound/drivers/mpu401/mpu401.c:   if ((err = 
pnp_register_driver(&snd_mpu401_pnp_driver)) >= 0) {

only old oss driver seem to use it, the other alsa driver use 
pnp_register_card_driver that seem only provide >= 0 res


Matthieu


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click

Re: mpu401_pnp

[Clemens Ladisch]

matthieu castet wrote:
> Clemens Ladisch wrote:
> > matthieu castet wrote:
> >
> >>Ok, I found a bug in your patch : when you call pnp_register_driver, you
> >>should check that it is >= 0. If not you shouldn't call
> >>pnp_unregister_driver.
> >
> > I just copied this from the other ALSA PnP drivers.  It seems those
> > have all the same bug.
>
> only old oss driver seem to use it, the other alsa driver use
> pnp_register_card_driver that seem only provide >= 0 res

It seems both pnp_register_driver() and pnp_register_card_driver()
can return a negative value (when CONFIG_PNP isn't set).  I'll fix it
when I've found out how the 2.2/2.4 wrappers behave.


Best regards,
Clemens



-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click

Re: mpu401_pnp

[matthieu castet]

Hi,

could you apply my patch in the cvs or if you don't like it, just 
correct the bug ?


Thanks,

Matthieu



-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click

Re: mpu401_pnp

[Clemens Ladisch]

matthieu castet wrote:
> attached a version more readable with less ifdef.

Applied, thanks.


Clemens



-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click