[Patch] sb_card.c cleanup

[Patch] sb_card.c cleanup

[Paul Laufer]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1154 bytes --]

Hey all,

Alex told me to redo my previous sb_card.c cleanup patch after he got his
into the kernel. So here is a new version, against 2.3.99pre3.

Overview: After Alex's last patch the various sb_init_* functions were
quite redundant, so I've written a generic sb_init function.  I also
reorganized sb_isapnp_list.  I raided the ALSA drivers for all the ISAPnP
codes for CTL and ESS chips and added them to the table (as I believe Alan
suggested). Also returns the 2.3.99pre2 dev->active behavior, which was
necessary to detect two of the same type of cards (Alex changed that in
his last patch). I cut out some redundant code in probe_sb(), and cleaned
up sb_isapnp_init(). Finally, I cleaned up the ISAPnP card detection
procedure by dumping the audio device detection procedure and just using
card detection (which should be okay since we have all the card ids).  
The overall result of this patch is a larger sb_isapnp_list with much less
duplicated code - which I believe to be a good trade off. Have a look and
please let me know what ya think, how it works for you, and/or if there is
something I missed.

Thanks again for your time,
Paul Laufer

[-- Attachment #2: Type: TEXT/PLAIN, Size: 18975 bytes --]

--- linux-devel-virgin/drivers/sound/sb_card.c	Sat Mar 25 15:52:34 2000
+++ linux/drivers/sound/sb_card.c	Mon Mar 27 02:13:14 2000
@@ -33,6 +33,9 @@
  *  Removed joystick and wavetable code, there are better places for them.
  *  Code cleanup plus some fixes. 
  *
+ * 27-03-2000 Major ISAPnP cleanup and reorganization.
+ *  Paul Laufer <pelaufer@csupomona.edu>
+ *
  */
 
 #include <linux/config.h>
@@ -126,14 +129,6 @@
 		}
 	}
 #endif
-
-	/* This is useless since is done by sb_dsp_detect - azummo */
-	
-	if (check_region(hw_config->io_base, 16))
-	{
-		printk(KERN_ERR "sb_card: I/O port 0x%x is already in use\n\n", hw_config->io_base);
-		return 0;
-	}
 	return sb_dsp_detect(hw_config, 0, 0);
 }
 
@@ -167,7 +162,6 @@
 #if defined CONFIG_ISAPNP || defined CONFIG_ISAPNP_MODULE
 static int isapnp	= 1;
 static int isapnpjump	= 0;
-static int activated	= 1;
 #else
 static int isapnp	= 0;
 #endif
@@ -203,6 +197,141 @@
 
 #if defined CONFIG_ISAPNP || defined CONFIG_ISAPNP_MODULE
 
+/* Please add new entries at the end of the table */
+static struct {char *name; unsigned short card_vendor, card_device, audio_vendor, audio_function, mpu_vendor, mpu_function;}
+sb_isapnp_list[] __initdata = {
+	{"Sound Blaster 16", 
+		ISAPNP_VENDOR('C','T','L'), ISAPNP_DEVICE(0x0024),
+		ISAPNP_VENDOR('C','T','L'), ISAPNP_FUNCTION(0x0031),
+		0,0 },
+	{"Sound Blaster 16", 
+		ISAPNP_VENDOR('C','T','L'), ISAPNP_DEVICE(0x0026), 
+		ISAPNP_VENDOR('C','T','L'), ISAPNP_FUNCTION(0x0031),
+		0,0},
+	{"Sound Blaster 16", 
+		ISAPNP_VENDOR('C','T','L'), ISAPNP_DEVICE(0x0027), 
+		ISAPNP_VENDOR('C','T','L'), ISAPNP_FUNCTION(0x0031),
+		0,0},
+	{"Sound Blaster 16", 
+		ISAPNP_VENDOR('C','T','L'), ISAPNP_DEVICE(0x0029), 
+		ISAPNP_VENDOR('C','T','L'), ISAPNP_FUNCTION(0x0031),
+		0,0},
+	{"Sound Blaster 16", 
+		ISAPNP_VENDOR('C','T','L'), ISAPNP_DEVICE(0x002b), 
+		ISAPNP_VENDOR('C','T','L'), ISAPNP_FUNCTION(0x0031),
+		0,0},
+	{"Sound Blaster Vibra16S", 
+		ISAPNP_VENDOR('C','T','L'), ISAPNP_DEVICE(0x0051), 
+		ISAPNP_VENDOR('C','T','L'), ISAPNP_FUNCTION(0x0001),
+		0,0},
+	{"Sound Blaster Vibra16C", 
+		ISAPNP_VENDOR('C','T','L'), ISAPNP_DEVICE(0x0070), 
+		ISAPNP_VENDOR('C','T','L'), ISAPNP_FUNCTION(0x0001),
+		0,0},
+	{"Sound Blaster Vibra16CL", 
+		ISAPNP_VENDOR('C','T','L'), ISAPNP_DEVICE(0x0080), 
+		ISAPNP_VENDOR('C','T','L'), ISAPNP_FUNCTION(0x0041),
+		0,0},
+	{"Sound Blaster Vibra16X", 
+		ISAPNP_VENDOR('C','T','L'), ISAPNP_DEVICE(0x00F0), 
+		ISAPNP_VENDOR('C','T','L'), ISAPNP_FUNCTION(0x0043),
+		0,0},
+	{"Sound Blaster AWE 32", 
+		ISAPNP_VENDOR('C','T','L'), ISAPNP_DEVICE(0x0039), 
+		ISAPNP_VENDOR('C','T','L'), ISAPNP_FUNCTION(0x0031),
+		0,0},
+	{"Sound Blaster AWE 32",
+		ISAPNP_VENDOR('C','T','L'), ISAPNP_DEVICE(0x0042), 
+		ISAPNP_VENDOR('C','T','L'), ISAPNP_FUNCTION(0x0031),
+		0,0},
+	{"Sound Blaster AWE 32",
+		ISAPNP_VENDOR('C','T','L'), ISAPNP_DEVICE(0x0043), 
+		ISAPNP_VENDOR('C','T','L'), ISAPNP_FUNCTION(0x0031),
+		0,0},
+	{"Sound Blaster AWE 32",
+		ISAPNP_VENDOR('C','T','L'), ISAPNP_DEVICE(0x0044),
+		ISAPNP_VENDOR('C','T','L'), ISAPNP_FUNCTION(0x0031),
+		0,0},
+	{"Sound Blaster AWE 32",
+		ISAPNP_VENDOR('C','T','L'), ISAPNP_DEVICE(0x0048), 
+		ISAPNP_VENDOR('C','T','L'), ISAPNP_FUNCTION(0x0031),
+		0,0},
+	{"Sound Blaster AWE 32",
+		ISAPNP_VENDOR('C','T','L'), ISAPNP_DEVICE(0x0054), 
+		ISAPNP_VENDOR('C','T','L'), ISAPNP_FUNCTION(0x0031),
+		0,0},
+	{"Sound Blaster AWE 32",
+		ISAPNP_VENDOR('C','T','L'), ISAPNP_DEVICE(0x009C), 
+		ISAPNP_VENDOR('C','T','L'), ISAPNP_FUNCTION(0x0041),
+		0,0},
+	{"Sound Blaster AWE 64",
+		ISAPNP_VENDOR('C','T','L'), ISAPNP_DEVICE(0x009D), 
+		ISAPNP_VENDOR('C','T','L'), ISAPNP_FUNCTION(0x0042),
+		0,0},
+	{"Sound Blaster AWE 64 Gold",
+		ISAPNP_VENDOR('C','T','L'), ISAPNP_DEVICE(0x009E), 
+		ISAPNP_VENDOR('C','T','L'), ISAPNP_FUNCTION(0x0044),
+		0,0},
+	{"Sound Blaster AWE 64",
+		ISAPNP_VENDOR('C','T','L'), ISAPNP_DEVICE(0x00C1), 
+		ISAPNP_VENDOR('C','T','L'), ISAPNP_FUNCTION(0x0042),
+		0,0},
+	{"Sound Blaster AWE 64",
+		ISAPNP_VENDOR('C','T','L'), ISAPNP_DEVICE(0x00C3), 
+		ISAPNP_VENDOR('C','T','L'), ISAPNP_FUNCTION(0x0045),
+		0,0},
+	{"Sound Blaster AWE 64",
+		ISAPNP_VENDOR('C','T','L'), ISAPNP_DEVICE(0x00C5), 
+		ISAPNP_VENDOR('C','T','L'), ISAPNP_FUNCTION(0x0045),
+		0,0},
+	{"Sound Blaster AWE 64",
+		ISAPNP_VENDOR('C','T','L'), ISAPNP_DEVICE(0x00C7), 
+		ISAPNP_VENDOR('C','T','L'), ISAPNP_FUNCTION(0x0045),
+		0,0},
+	{"Sound Blaster AWE 64",
+		ISAPNP_VENDOR('C','T','L'), ISAPNP_DEVICE(0x00E4), 
+		ISAPNP_VENDOR('C','T','L'), ISAPNP_FUNCTION(0x0045),
+		0,0},
+	{"ESS 1868",
+		ISAPNP_VENDOR('E','S','S'), ISAPNP_DEVICE(0x1868), 
+		ISAPNP_VENDOR('E','S','S'), ISAPNP_FUNCTION(0x1868),
+		0,0},
+	{"ESS 1868",
+		ISAPNP_VENDOR('E','S','S'), ISAPNP_DEVICE(0x1868), 
+		ISAPNP_VENDOR('E','S','S'), ISAPNP_FUNCTION(0x8611),
+		0,0},
+	{"ESS 1869 PnP AudioDrive", 
+		ISAPNP_VENDOR('E','S','S'), ISAPNP_DEVICE(0x0003), 
+		ISAPNP_VENDOR('E','S','S'), ISAPNP_FUNCTION(0x1869),
+		0,0},
+	{"ESS 1869",
+		ISAPNP_VENDOR('E','S','S'), ISAPNP_DEVICE(0x1869), 
+		ISAPNP_VENDOR('E','S','S'), ISAPNP_FUNCTION(0x1869),
+		0,0},
+	{"ESS 1878",
+		ISAPNP_VENDOR('E','S','S'), ISAPNP_DEVICE(0x1878), 
+		ISAPNP_VENDOR('E','S','S'), ISAPNP_FUNCTION(0x1878),
+		0,0},
+	{"ESS 1879",
+		ISAPNP_VENDOR('E','S','S'), ISAPNP_DEVICE(0x1879), 
+		ISAPNP_VENDOR('E','S','S'), ISAPNP_FUNCTION(0x1879),
+		0,0},
+	{"CMI 8330 SoundPRO",
+		ISAPNP_VENDOR('C','M','I'), ISAPNP_DEVICE(0x0001), 
+		ISAPNP_VENDOR('@','X','@'), ISAPNP_FUNCTION(0x0001),
+		ISAPNP_VENDOR('@','H','@'), ISAPNP_FUNCTION(0x0001)},
+	{"Diamond DT0197H",
+		ISAPNP_VENDOR('R','W','B'), ISAPNP_DEVICE(0x1688), 
+		ISAPNP_VENDOR('@','@','@'), ISAPNP_FUNCTION(0x0001),
+		ISAPNP_VENDOR('@','X','@'), ISAPNP_FUNCTION(0x0001)},
+	{"ALS 100",
+	       	ISAPNP_VENDOR('A','L','S'), ISAPNP_DEVICE(0x0001), 
+		ISAPNP_VENDOR('@','@','@'), ISAPNP_FUNCTION(0x0001),
+		ISAPNP_VENDOR('@','X','@'), ISAPNP_FUNCTION(0x0001)},
+	{0}
+};
+
+
 /* That's useful. */
 
 #define show_base(devname, resname, resptr) printk(KERN_INFO "sb: %s %s base located at %#lx\n", devname, resname, (resptr)->start)
@@ -213,11 +342,12 @@
 
 	/* Device already active? Let's use it */
 
+	/* Umm, can't do that if we want to use two of them since
+	   the driver will keep tripping over the first one. */
+
 	if(dev->active)
-	{
-		activated = 0;
-		return(dev);
-	}
+		return(NULL);
+	
 	if((err = dev->activate(dev)) < 0)
 	{
 		printk(KERN_ERR "sb: %s %s config failed (out of resources?)[%d]\n", devname, resname, err);
@@ -229,260 +359,66 @@
 	return(dev);
 }
 
-/* Card's specific initialization functions
- */
-
-static struct pci_dev *sb_init_generic(struct pci_bus *bus, struct pci_dev *card, struct address_info *hw_config, struct address_info *mpu_config)
+static struct pci_dev *sb_init(struct pci_bus *bus, struct address_info *hw_config, struct address_info *mpu_config, int slot)
 {
-	if((sb_dev = isapnp_find_dev(bus, card->vendor, card->device, NULL)))
-	{
-		sb_dev->prepare(sb_dev);
+	/* Configure Audio device */
 
-		if((sb_dev = activate_dev("Soundblaster", "sb", sb_dev)))
-		{
-			hw_config->io_base 	= sb_dev->resource[0].start;
-			hw_config->irq 		= sb_dev->irq_resource[0].start;
-			hw_config->dma 		= sb_dev->dma_resource[0].start;
-			hw_config->dma2 	= sb_dev->dma_resource[1].start;
-			mpu_config->io_base	= sb_dev->resource[1].start;
-		}
-	}
-	return(sb_dev);
-}
-
-static struct pci_dev *sb_init_ess(struct pci_bus *bus, struct pci_dev *card, struct address_info *hw_config, struct address_info *mpu_config)
-{
-	if((sb_dev = isapnp_find_dev(bus, card->vendor, card->device, NULL)))
+	if((sb_dev = isapnp_find_dev(bus, sb_isapnp_list[slot].audio_vendor, sb_isapnp_list[slot].audio_function, NULL)))
 	{
 		sb_dev->prepare(sb_dev);
 
-		if((sb_dev = activate_dev("ESS", "sb", sb_dev)))
-		{
-			hw_config->io_base 	= sb_dev->resource[0].start;
-			hw_config->irq 		= sb_dev->irq_resource[0].start;
-			hw_config->dma 		= sb_dev->dma_resource[0].start;
-			hw_config->dma2		= sb_dev->dma_resource[1].start;
-			mpu_config->io_base	= sb_dev->resource[2].start;
-		}
-	}
-	return(sb_dev);
-}
-
-static struct pci_dev *sb_init_cmi(struct pci_bus *bus, struct pci_dev *card, struct address_info *hw_config, struct address_info *mpu_config)
-{
-	/* 
-	 *  The CMI8330/C3D is a very 'stupid' chip... where did they get al those @@@ ?
-	 *  It's ISAPnP section is badly designed and has many flaws, i'll do my best
-	 *  to workaround them. I strongly suggest you to buy a real soundcard.
-	 *  The CMI8330 on my motherboard has also the bad habit to activate 
-	 *  the rear channel of my amplifier instead of the front one.
-	 */
-
-	/*  @X@0001:Soundblaster.
-	 */
-
-	if((sb_dev = isapnp_find_dev(bus,
-		ISAPNP_VENDOR('@','X','@'), ISAPNP_FUNCTION(0x0001), NULL)))
-	{
-		sb_dev->prepare(sb_dev);
-		
-		if((sb_dev = activate_dev("CMI8330", "sb", sb_dev)))
+		if((sb_dev = activate_dev(sb_isapnp_list[slot].name, "sb", sb_dev)))
 		{
 			hw_config->io_base 	= sb_dev->resource[0].start;
 			hw_config->irq 		= sb_dev->irq_resource[0].start;
 			hw_config->dma 		= sb_dev->dma_resource[0].start;
 			hw_config->dma2 	= sb_dev->dma_resource[1].start;
+		} else
+			return(NULL);
+	} else
+		return(NULL);
 
-			show_base("CMI8330", "sb", &sb_dev->resource[0]);
-		}
-
-		if(!sb_dev) return(NULL);
-	}
-	else
-		printk(KERN_ERR "sb: CMI8330 panic: sb base not found\n");
-
-	/*  @H@0001:mpu
-	 */
-
-	if((mpu_dev = isapnp_find_dev(bus,
-		ISAPNP_VENDOR('@','H','@'), ISAPNP_FUNCTION(0x0001), NULL)))
-	{
-		mpu_dev->prepare(mpu_dev);
-
-		/*  This disables the interrupt on this resource. Do we need it ?
-		 */
-
-		mpu_dev->irq_resource[0].flags = 0;
-
-		if((mpu_dev = activate_dev("CMI8330", "mpu", mpu_dev)))
-		{
-			show_base("CMI8330", "mpu", &mpu_dev->resource[0]);
-			mpu_config->io_base = mpu_dev->resource[0].start;
-		}
-	}
-	else
-		printk(KERN_ERR "sb: CMI8330 panic: mpu not found\n");
-
-	printk(KERN_INFO "sb: CMI8330 mail reports to Alessandro Zummo <azummo@ita.flashnet.it>\n");
-
-	return(sb_dev);
-}
-
-static struct pci_dev *sb_init_diamond(struct pci_bus *bus, struct pci_dev *card, struct address_info *hw_config, struct address_info *mpu_config)
-{
-	/* 
-	 * Diamonds DT0197H
-	 * very similar to the CMI8330 above
-	 */
-
-	/*  @@@0001:Soundblaster.
-	 */
-
-	if((sb_dev = isapnp_find_dev(bus,
-				ISAPNP_VENDOR('@','@','@'), ISAPNP_FUNCTION(0x0001), NULL)))
-	{
-		sb_dev->prepare(sb_dev);
-		
-		if((sb_dev = activate_dev("DT0197H", "sb", sb_dev)))
-		{
-			hw_config->io_base 	= sb_dev->resource[0].start;
-			hw_config->irq 		= sb_dev->irq_resource[0].start;
-			hw_config->dma 		= sb_dev->dma_resource[0].start;
-			hw_config->dma2 	= -1;
-
-			show_base("DT0197H", "sb", &sb_dev->resource[0]);
-		}
-
-		if(!sb_dev) return(NULL);
-	}
-	else
-		printk(KERN_ERR "sb: DT0197H panic: sb base not found\n");
-
-	/*  @X@0001:mpu
-	 */
-
-	if((mpu_dev = isapnp_find_dev(bus,
-				ISAPNP_VENDOR('@','X','@'), ISAPNP_FUNCTION(0x0001), NULL)))
-	{
-		mpu_dev->prepare(mpu_dev);
-
-		if((mpu_dev = activate_dev("DT0197H", "mpu", mpu_dev)))
-		{
-			show_base("DT0197H", "mpu", &mpu_dev->resource[0]);
-			mpu_config->io_base = mpu_dev->resource[0].start;
-		}
-	}
-	else
-		printk(KERN_ERR "sb: DT0197H panic: mpu not found\n");
-
-	printk(KERN_INFO "sb: DT0197H mail reports to Torsten Werner <twerner@intercomm.de>\n");
-
-	return(sb_dev);
-}
-
-static struct pci_dev *sb_init_als(struct pci_bus *bus, struct pci_dev *card, struct address_info *hw_config, struct address_info *mpu_config)
-{
-	/* 
-	 * ALS100
-	 * very similar to both ones above above
-	 */
-
-	/*  @@@0001:Soundblaster.
-	 */
-
-	if((sb_dev = isapnp_find_dev(bus,
-				ISAPNP_VENDOR('@','@','@'), ISAPNP_FUNCTION(0x0001), NULL)))
-	{
-		sb_dev->prepare(sb_dev);
-		
-		if((sb_dev = activate_dev("ALS100", "sb", sb_dev)))
-		{
-			hw_config->io_base 	= sb_dev->resource[0].start;
-			hw_config->irq 		= sb_dev->irq_resource[0].start;
-			hw_config->dma 		= sb_dev->dma_resource[1].start;
-			hw_config->dma2 	= sb_dev->dma_resource[0].start;
-
-			show_base("ALS100", "sb", &sb_dev->resource[0]);
-		}
-
-		if(!sb_dev) return(NULL);
+	/* Cards with MPU part of Audio device */
+	
+	if(!sb_isapnp_list[slot].mpu_vendor) {
+		mpu_config->io_base	= sb_dev->resource[1].start;
+		return(sb_dev);
 	}
-	else
-		printk(KERN_ERR "sb: ALS100 panic: sb base not found\n");
 
-	/*  @X@0001:mpu
-	 */
+	/* Cards with separate MPU device */
 
-	if((mpu_dev = isapnp_find_dev(bus,
-				ISAPNP_VENDOR('@','X','@'), ISAPNP_FUNCTION(0x0001), NULL)))
+	if((mpu_dev = isapnp_find_dev(bus, sb_isapnp_list[slot].mpu_vendor, sb_isapnp_list[slot].mpu_function, NULL)))
 	{
 		mpu_dev->prepare(mpu_dev);
 
-		if((mpu_dev = activate_dev("ALS100", "mpu", mpu_dev)))
-		{
-			show_base("ALS100", "mpu", &mpu_dev->resource[0]);
+		if((mpu_dev = activate_dev(sb_isapnp_list[slot].name, "mpu", mpu_dev)))
 			mpu_config->io_base = mpu_dev->resource[0].start;
-		}
 	}
 	else
-		printk(KERN_ERR "sb: ALS100 panic: mpu not found\n");
-
-	printk(KERN_INFO "sb: ALS100 mail reports to Torsten Werner <twerner@intercomm.de>\n");
-
+		printk(KERN_ERR "sb: %s panic: mpu not found\n", sb_isapnp_list[slot].name);
+	
 	return(sb_dev);
 }
 
-#define SBF_DEV	0x01 /* Please notice that cards without this flag are on the top in the list */
-
-
-static struct { unsigned short vendor, function, flags; struct pci_dev * (*initfunc)(struct pci_bus *, struct pci_dev *, struct address_info *, struct address_info *); char *name; }
-sb_isapnp_list[] __initdata = {
-	{ISAPNP_VENDOR('C','M','I'), ISAPNP_FUNCTION(0x0001), 0,	&sb_init_cmi,		"CMI 8330 SoundPRO" },
-	{ISAPNP_VENDOR('R','W','B'), ISAPNP_FUNCTION(0x1688), 0,	&sb_init_diamond,	"Diamond DT0197H" },
-	{ISAPNP_VENDOR('A','L','S'), ISAPNP_FUNCTION(0x0001), 0,	&sb_init_als,		"ALS 100" },
-	{ISAPNP_VENDOR('C','T','L'), ISAPNP_FUNCTION(0x0001), SBF_DEV,	&sb_init_generic,	"Sound Blaster 16" },
-	{ISAPNP_VENDOR('C','T','L'), ISAPNP_FUNCTION(0x0031), SBF_DEV,	&sb_init_generic,	"Sound Blaster 16" },
-	{ISAPNP_VENDOR('C','T','L'), ISAPNP_FUNCTION(0x0041), SBF_DEV,	&sb_init_generic,	"Sound Blaster 16" },
-	{ISAPNP_VENDOR('C','T','L'), ISAPNP_FUNCTION(0x0042), SBF_DEV,	&sb_init_generic,	"Sound Blaster 16" },
-	{ISAPNP_VENDOR('C','T','L'), ISAPNP_FUNCTION(0x0043), SBF_DEV,	&sb_init_generic,	"Sound Blaster 16" },
-	{ISAPNP_VENDOR('C','T','L'), ISAPNP_FUNCTION(0x0045), SBF_DEV,	&sb_init_generic,	"Sound Blaster 16" },
-	{ISAPNP_VENDOR('E','S','S'), ISAPNP_FUNCTION(0x0968), SBF_DEV,	&sb_init_ess,		"ESS 1688" },
-	{ISAPNP_VENDOR('E','S','S'), ISAPNP_FUNCTION(0x1868), SBF_DEV,	&sb_init_ess,		"ESS 1868" },
-	{ISAPNP_VENDOR('E','S','S'), ISAPNP_FUNCTION(0x8611), SBF_DEV,	&sb_init_ess,		"ESS 1868" },
-	{ISAPNP_VENDOR('E','S','S'), ISAPNP_FUNCTION(0x1869), SBF_DEV,	&sb_init_ess,		"ESS 1869" },
-	{ISAPNP_VENDOR('E','S','S'), ISAPNP_FUNCTION(0x1878), SBF_DEV,	&sb_init_ess,		"ESS 1878" },
-	{ISAPNP_VENDOR('E','S','S'), ISAPNP_FUNCTION(0x1879), SBF_DEV,	&sb_init_ess,		"ESS 1879" },
-	{0}
-};
-
 static int __init sb_isapnp_init(struct address_info *hw_config, struct address_info *mpu_config, struct pci_bus *bus, struct pci_dev *card, int slot)
 {
-	struct pci_dev *idev = NULL;
+	char *busname = bus->name[0] ? bus->name : sb_isapnp_list[slot].name;
 
-	/* You missed the init func? That's bad. */
-	if(sb_isapnp_list[slot].initfunc)
-	{
-		char *busname = bus->name[0] ? bus->name : sb_isapnp_list[slot].name;
+	printk(KERN_INFO "sb: %s detected\n", busname); 
 
-		printk(KERN_INFO "sb: %s detected\n", busname); 
+	/* Initialize this baby. */
 
-		/* Initialize this baby. */
-
-		if((idev = sb_isapnp_list[slot].initfunc(bus, card, hw_config, mpu_config)))
-		{
-			/* We got it. */
-
-			printk(KERN_NOTICE "sb: ISAPnP reports '%s' at i/o %#x, irq %d, dma %d, %d\n",
-			       busname,
-			       hw_config->io_base, hw_config->irq, hw_config->dma,
-			       hw_config->dma2);
-			return 1;
-		}
-		else
-			printk(KERN_INFO "sb: Failed to initialize %s\n", busname);
+	if(sb_init(bus, hw_config, mpu_config, slot)) {
+		/* We got it. */
+		
+		printk(KERN_NOTICE "sb: ISAPnP reports '%s' at i/o %#x, irq %d, dma %d, %d\n",
+		       busname,
+		       hw_config->io_base, hw_config->irq, hw_config->dma,
+		       hw_config->dma2);
+		return 1;
 	}
 	else
-		printk(KERN_ERR "sb: Bad entry in sb_card.c PnP table\n");
+		printk(KERN_INFO "sb: Failed to initialize %s\n", busname);
 
 	return 0;
 }
@@ -498,50 +434,24 @@
 	int i;
 
 	/* Count entries in sb_isapnp_list */
-	for (i = 0; sb_isapnp_list[i].vendor != 0; i++);
+	for (i = 0; sb_isapnp_list[i].card_vendor != 0; i++);
 
 	/* Check and adjust isapnpjump */
-	if( isapnpjump < 0 || isapnpjump > ( i - 1 ) )
-	{
+	if( isapnpjump < 0 || isapnpjump > ( i - 1 ) ) {
 		printk(KERN_ERR "sb: Valid range for isapnpjump is 0-%d. Adjusted to 0.\n", i-1);
 		isapnpjump = 0;
 	}
 	
-	for (i = isapnpjump; sb_isapnp_list[i].vendor != 0; i++) {
-
-		if(!(sb_isapnp_list[i].flags & SBF_DEV))
-		{
-			struct pci_bus *bus = NULL;
+	for (i = isapnpjump; sb_isapnp_list[i].card_vendor != 0; i++) {
+		struct pci_bus *bus = NULL;
 				
-			while ((bus = isapnp_find_card(
-					sb_isapnp_list[i].vendor,
-					sb_isapnp_list[i].function,
-					bus))) {
+		while ((bus = isapnp_find_card(
+				sb_isapnp_list[i].card_vendor,
+				sb_isapnp_list[i].card_device,
+				bus))) {
 	
-				if(sb_isapnp_init(hw_config, mpu_config, bus, NULL, i))
-					return 0;
-			}
-		}
-	}
-
-	/*  No cards found. I'll try now to search inside every card for a logical device
-	 *  that matches any entry marked with SBF_DEV in the table.
-	 */
-
-	for (i = isapnpjump; sb_isapnp_list[i].vendor != 0; i++) {
-
-		if(sb_isapnp_list[i].flags & SBF_DEV)
-		{
-			struct pci_dev *card = NULL;
-
-			while ((card = isapnp_find_dev(NULL,
-					sb_isapnp_list[i].vendor,
-					sb_isapnp_list[i].function,
-					card))) {
-
-				if(sb_isapnp_init(hw_config, mpu_config, card->bus, card, i))
-					return 0;
-			}
+			if(sb_isapnp_init(hw_config, mpu_config, bus, NULL, i))
+				return 0;
 		}
 	}
 
@@ -601,11 +511,8 @@
 	SOUND_LOCK_END;
 
 #if defined CONFIG_ISAPNP || defined CONFIG_ISAPNP_MODULE			
-	if(activated)
-	{
-		if(sb_dev) 	sb_dev->deactivate(sb_dev);
-		if(mpu_dev) 	mpu_dev->deactivate(mpu_dev);
-	}
+	if(sb_dev) 	sb_dev->deactivate(sb_dev);
+	if(mpu_dev) 	mpu_dev->deactivate(mpu_dev);
 #endif
 }
 

Re: [Patch] sb_card.c cleanup

[Gaël Quéri]

On Mon, Mar 27, 2000 at 02:19:17AM -0800, Paul Laufer wrote:
> Hey all,
> 
> Alex told me to redo my previous sb_card.c cleanup patch after he got his
> into the kernel. So here is a new version, against 2.3.99pre3.
> 
> Overview: After Alex's last patch the various sb_init_* functions were
> quite redundant, so I've written a generic sb_init function.  I also
> reorganized sb_isapnp_list.  I raided the ALSA drivers for all the ISAPnP
> codes for CTL and ESS chips and added them to the table (as I believe Alan
> suggested). Also returns the 2.3.99pre2 dev->active behavior, which was
> necessary to detect two of the same type of cards (Alex changed that in
> his last patch). I cut out some redundant code in probe_sb(), and cleaned
> up sb_isapnp_init(). Finally, I cleaned up the ISAPnP card detection
> procedure by dumping the audio device detection procedure and just using
> card detection (which should be okay since we have all the card ids).  
> The overall result of this patch is a larger sb_isapnp_list with much less
> duplicated code - which I believe to be a good trade off. Have a look and
> please let me know what ya think, how it works for you, and/or if there is
> something I missed.

That's great !!! sb_card.c was a bloat of nearly identical sb_init_*
functions...

But the problem is the nearly... Some cards are really broken in
the sense that irq_resource[0] != dma and irq_resource[1] != dma2...
For example the ALS007 doesn't have a dma2 but that's
possible to fix it...
But in the vanilla sb_card.c from 2.3.99-pre3 you can see that the
ALS100 has dma = irq_resource[1] and dma2 = irq_resource[0]...
I really hope this was a typo because this would oblige us to put
some new members in the struct of isapnp_list...

For the ALS007 the problem is that dma2 is set to 0, which is a valid
dma. It works for me because I don't have anything using dma 0 but
it's definitely not a good thing...
The solution would be to set dma2 to -1 when we are sure it is not
used but I didn't found an obvious way to do it...
What I propose is replacing

	hw_config->dma2 = sb_dev->dma_resource[1].start;

by

	if (sb_dev->dma_resource[1].child)
		hw_config->dma2 = sb_dev->dma_resource[1].start;
	else
		hw_config->dma2 = -1;


Here I assume that normally dma_resource[1] is the child of dma_resource[0]
but I can't test it... all my devices have only one irq and one dma.
So please someone tell me if that portion of code is warranted to pick
the dma2 in ALL the cards which have one, or if there is a better way
to do it


thanks, gael

Re: [Patch] sb_card.c cleanup

[Peter Rasmussen]

This all sounds great (no pun ... :-) and I would like to know if it while
doing this would be possible to allow the second DMA channel to be 8bit and 
not requiring it to be 16bit as it is today?

I and a fair amount of other people it seems, have SB Vibra cards that look
like SB16 cards, but have been handicapped by having two 8bit DMA channels 
instead of one 8bit and one 16bit. I need the second DMA channel for duplex.

Thank you very much,

Peter

gqueri@mail.dotcom.fr wrote:
>
>On Mon, Mar 27, 2000 at 02:19:17AM -0800, Paul Laufer wrote:
>> Hey all,
>> 
>> Alex told me to redo my previous sb_card.c cleanup patch after he got his
>> into the kernel. So here is a new version, against 2.3.99pre3.
>> 
>> Overview: After Alex's last patch the various sb_init_* functions were
>> quite redundant, so I've written a generic sb_init function.  I also
>> reorganized sb_isapnp_list.  I raided the ALSA drivers for all the ISAPnP
>> codes for CTL and ESS chips and added them to the table (as I believe Alan
>> suggested). Also returns the 2.3.99pre2 dev->active behavior, which was
>> necessary to detect two of the same type of cards (Alex changed that in
>> his last patch). I cut out some redundant code in probe_sb(), and cleaned
>> up sb_isapnp_init(). Finally, I cleaned up the ISAPnP card detection
>> procedure by dumping the audio device detection procedure and just using
>> card detection (which should be okay since we have all the card ids).  
>> The overall result of this patch is a larger sb_isapnp_list with much less
>> duplicated code - which I believe to be a good trade off. Have a look and
>> please let me know what ya think, how it works for you, and/or if there is
>> something I missed.
>
>That's great !!! sb_card.c was a bloat of nearly identical sb_init_*
>functions...
>
>But the problem is the nearly... Some cards are really broken in
>the sense that irq_resource[0] != dma and irq_resource[1] != dma2...
>For example the ALS007 doesn't have a dma2 but that's
>possible to fix it...
>But in the vanilla sb_card.c from 2.3.99-pre3 you can see that the
>ALS100 has dma = irq_resource[1] and dma2 = irq_resource[0]...
>I really hope this was a typo because this would oblige us to put
>some new members in the struct of isapnp_list...
>
>For the ALS007 the problem is that dma2 is set to 0, which is a valid
>dma. It works for me because I don't have anything using dma 0 but
>it's definitely not a good thing...
>The solution would be to set dma2 to -1 when we are sure it is not
>used but I didn't found an obvious way to do it...
>What I propose is replacing
>
>	hw_config->dma2 = sb_dev->dma_resource[1].start;
>
>by
>
>	if (sb_dev->dma_resource[1].child)
>		hw_config->dma2 = sb_dev->dma_resource[1].start;
>	else
>		hw_config->dma2 = -1;
>
>
>Here I assume that normally dma_resource[1] is the child of dma_resource[0]
>but I can't test it... all my devices have only one irq and one dma.
>So please someone tell me if that portion of code is warranted to pick
>the dma2 in ALL the cards which have one, or if there is a better way
>to do it
>
>
>thanks, gael
>
>

Re: [Patch] sb_card.c cleanup

[Paul Laufer]

On Mon, 27 Mar 2000, [iso-8859-15] Gaël Quéri wrote:

> On Mon, Mar 27, 2000 at 02:19:17AM -0800, Paul Laufer wrote:
> > Hey all,
> > 
> > Alex told me to redo my previous sb_card.c cleanup patch after he got his
> > into the kernel. So here is a new version, against 2.3.99pre3.
> > 
> > Overview: After Alex's last patch the various sb_init_* functions were
> > quite redundant, so I've written a generic sb_init function.  I also
> > reorganized sb_isapnp_list.  I raided the ALSA drivers for all the ISAPnP
> > codes for CTL and ESS chips and added them to the table (as I believe Alan
> > suggested). Also returns the 2.3.99pre2 dev->active behavior, which was
> > necessary to detect two of the same type of cards (Alex changed that in
> > his last patch). I cut out some redundant code in probe_sb(), and cleaned
> > up sb_isapnp_init(). Finally, I cleaned up the ISAPnP card detection
> > procedure by dumping the audio device detection procedure and just using
> > card detection (which should be okay since we have all the card ids).  
> > The overall result of this patch is a larger sb_isapnp_list with much less
> > duplicated code - which I believe to be a good trade off. Have a look and
> > please let me know what ya think, how it works for you, and/or if there is
> > something I missed.
> 
> That's great !!! sb_card.c was a bloat of nearly identical sb_init_*
> functions...
> 
> But the problem is the nearly... Some cards are really broken in
> the sense that irq_resource[0] != dma and irq_resource[1] != dma2...
> For example the ALS007 doesn't have a dma2 but that's
> possible to fix it...
> 
> But in the vanilla sb_card.c from 2.3.99-pre3 you can see that the
> ALS100 has dma = irq_resource[1] and dma2 = irq_resource[0]...   
> I really hope this was a typo because this would oblige us to put
> some new members in the struct of isapnp_list...

Hrm, looking at the ALSA als100 driver, they use dma_resource[1] = dma and
dma_resource[0] = dma2:

        snd_dma8[dev] = pdev->dma_resource[1].start;
        snd_dma16[dev] = pdev->dma_resource[0].start;

So, no typo. I guess the only thing to do is add another entry to
sb_isapnp_list... The ALS110, 120, and RTL3000 are the same way, it seems.
Do you know about the ALS200? It was in your diff but not mentioned in the
alsa driver, so I'm not sure if it has the same "backwardness" as the
ALS100.

> For the ALS007 the problem is that dma2 is set to 0, which is a valid
> dma. It works for me because I don't have anything using dma 0 but
> it's definitely not a good thing...
> The solution would be to set dma2 to -1 when we are sure it is not
> used but I didn't found an obvious way to do it...
> What I propose is replacing
> 
> 	hw_config->dma2 = sb_dev->dma_resource[1].start;
> 
> by
> 
> 	if (sb_dev->dma_resource[1].child)
> 		hw_config->dma2 = sb_dev->dma_resource[1].start;
> 	else
> 		hw_config->dma2 = -1;
> 
> 
> Here I assume that normally dma_resource[1] is the child of dma_resource[0]
> but I can't test it... all my devices have only one irq and one dma.
> So please someone tell me if that portion of code is warranted to pick
> the dma2 in ALL the cards which have one, or if there is a better way
> to do it
> 
> 
> thanks, gael

Well, hrm. Looking back over 2.3.99pre3 vanilla I that I missed this
completely. I should have paid more attention. I also see that the ESS and
CTL cards swap locations of the fm and mpu resource locations. I've just
fixed that with more isapnp_list members . . .

It does not appear that isapnp sets resource.child or resource.sibling.
I'll just add another member to isapnp_list. . .

Dangit, now things are getting complicated again. Still, I think
maintaining a table is easier than maintaining a mess of sb_init
functions.

Anyway, I'll get out a new patch soon.

Thanks!
Paul Laufer