[patch, 2.5] move snd_legacy_find_free_ioport to opti92x-ad1848.c

[patch, 2.5] move snd_legacy_find_free_ioport to opti92x-ad1848.c

[Marcus Alanen]

Moves the snd_legacy_find_free_ioport definition to opti92x-ad1848.c, 
since it is the only user.

#
# create_patch: move_snd_legacy_find_free_ioport-2002-12-30-A.patch
# Date: Mon Dec 30 21:13:59 EET 2002
#
diff -Naurd --exclude-from=/home/maalanen/linux/base/diff_exclude linus-2.5.53/include/sound/initval.h initval-2.5.53/include/sound/initval.h
--- linus-2.5.53/include/sound/initval.h	Fri Dec 27 00:04:28 2002
+++ initval-2.5.53/include/sound/initval.h	Mon Dec 30 19:11:42 2002
@@ -97,18 +97,6 @@
 }
 #endif
 
-#ifdef SNDRV_LEGACY_FIND_FREE_IOPORT
-static long snd_legacy_find_free_ioport(long *port_table, long size)
-{
-	while (*port_table != -1) {
-		if (!check_region(*port_table, size))
-			return *port_table;
-		port_table++;
-	}
-	return -1;
-}
-#endif
-
 #ifdef SNDRV_LEGACY_FIND_FREE_IRQ
 #include <linux/interrupt.h>
 
diff -Naurd --exclude-from=/home/maalanen/linux/base/diff_exclude linus-2.5.53/sound/isa/opti9xx/opti92x-ad1848.c initval-2.5.53/sound/isa/opti9xx/opti92x-ad1848.c
--- linus-2.5.53/sound/isa/opti9xx/opti92x-ad1848.c	Thu Oct 31 20:24:38 2002
+++ initval-2.5.53/sound/isa/opti9xx/opti92x-ad1848.c	Mon Dec 30 19:13:17 2002
@@ -47,7 +47,6 @@
 #endif	/* CS4231 */
 #include <sound/mpu401.h>
 #include <sound/opl3.h>
-#define SNDRV_LEGACY_FIND_FREE_IOPORT
 #define SNDRV_LEGACY_FIND_FREE_IRQ
 #define SNDRV_LEGACY_FIND_FREE_DMA
 #define SNDRV_GET_ID
@@ -323,6 +322,16 @@
 	"82C930",	"82C931",	"82C933"
 };
 
+
+static long snd_legacy_find_free_ioport(long *port_table, long size)
+{
+	while (*port_table != -1) {
+		if (!check_region(*port_table, size))
+			return *port_table;
+		port_table++;
+	}
+	return -1;
+}
 
 static int __init snd_opti9xx_init(opti9xx_t *chip, unsigned short hardware)
 {

[patch, 2.5] opti92x-ad1848 one check_region fixup

[Marcus Alanen]

Initializes the variables (the chip->xxx stuff) before calling 
anything else. snd_legacy_find_free_ioport() uses request_region now, 
so remember to release regions in the private freeing routine 
snd_card_opti9xx_free().

Note how I changed it to return SNDRV_AUTO_PORT instead of -1, I'm 
not sure if they are guaranteed to be the same, so I changed it 
instead explicitely.

No other snd_legacy_find_free_ioport users in this file or elsewhere 
in the kernel.

#
# create_patch: opti_1-2002-12-30-A.patch
# Date: Mon Dec 30 22:21:50 EET 2002
#
diff -Naurd --exclude-from=/home/maalanen/linux/base/diff_exclude opti_original-2.5.53/sound/isa/opti9xx/opti92x-ad1848.c opti_1-2.5.53/sound/isa/opti9xx/opti92x-ad1848.c
--- opti_original-2.5.53/sound/isa/opti9xx/opti92x-ad1848.c	Mon Dec 30 20:00:42 2002
+++ opti_1-2.5.53/sound/isa/opti9xx/opti92x-ad1848.c	Mon Dec 30 20:19:47 2002
@@ -326,11 +326,11 @@
 static long snd_legacy_find_free_ioport(long *port_table, long size)
 {
 	while (*port_table != -1) {
-		if (!check_region(*port_table, size))
+		if (request_region(*port_table, size, "opti92x-ad1848"))
 			return *port_table;
 		port_table++;
 	}
-	return -1;
+	return SNDRV_AUTO_PORT;
 }
 
 static int __init snd_opti9xx_init(opti9xx_t *chip, unsigned short hardware)
@@ -1911,6 +1911,10 @@
 #ifdef __ISAPNP__
 		snd_card_opti9xx_deactivate(chip);
 #endif	/* __ISAPNP__ */
+		if (chip->wss_base != SNDRV_AUTO_PORT)
+			release_region(chip->wss_base, 4);
+		if (chip->mpu_port != SNDRV_AUTO_PORT)
+			release_region(chip->mpu_port, 2);
 		if (chip->res_mc_base) {
 			release_resource(chip->res_mc_base);
 			kfree_nocheck(chip->res_mc_base);
@@ -1956,6 +1960,16 @@
 	card->private_free = snd_card_opti9xx_free;
 	chip = (opti9xx_t *)card->private_data;
 
+	chip->wss_base = port;
+	chip->fm_port = fm_port;
+	chip->mpu_port = mpu_port;
+	chip->irq = irq;
+	chip->mpu_irq = mpu_irq;
+	chip->dma1 = dma1;
+#if defined(CS4231) || defined(OPTi93X)
+	chip->dma2 = dma2;
+#endif
+
 #ifdef __ISAPNP__
 	if (isapnp && (hw = snd_card_opti9xx_isapnp(chip)) > 0) {
 		switch (hw) {
@@ -1994,28 +2008,18 @@
 		return -ENOMEM;
 	}
 
-	chip->wss_base = port;
-	chip->fm_port = fm_port;
-	chip->mpu_port = mpu_port;
-	chip->irq = irq;
-	chip->mpu_irq = mpu_irq;
-	chip->dma1 = dma1;
-#if defined(CS4231) || defined(OPTi93X)
-	chip->dma2 = dma2;
-#endif
-
 #ifdef __ISAPNP__
 	if (!isapnp) {
 #endif
 	if (chip->wss_base == SNDRV_AUTO_PORT) {
-		if ((chip->wss_base = snd_legacy_find_free_ioport(possible_ports, 4)) < 0) {
+		if ((chip->wss_base = snd_legacy_find_free_ioport(possible_ports, 4)) == SNDRV_AUTO_PORT) {
 			snd_card_free(card);
 			snd_printk("unable to find a free WSS port\n");
 			return -EBUSY;
 		}
 	}
 	if (chip->mpu_port == SNDRV_AUTO_PORT) {
-		if ((chip->mpu_port = snd_legacy_find_free_ioport(possible_mpu_ports, 2)) < 0) {
+		if ((chip->mpu_port = snd_legacy_find_free_ioport(possible_mpu_ports, 2)) == SNDRV_AUTO_PORT) {
 			snd_card_free(card);
 			snd_printk("unable to find a free MPU401 port\n");
 			return -EBUSY;

Re: [patch, 2.5] opti92x-ad1848 second check_region fixup

[Marcus Alanen]

Changes check_region in snd_card_opti9xx_detect() to request_region,
with appropriate release_region. opti9xx_free() releases this region 
using chip->res_mc_base.

Since the _detect case now uses request_region, we can't do the same 
request_region afterwards, that would fail. So we move it inside the 
__ISAPNP__ case.

#
# create_patch: opti_2-2002-12-30-A.patch
# Date: Mon Dec 30 22:53:33 EET 2002
#
diff -Naurd --exclude-from=/home/maalanen/linux/base/diff_exclude opti_1-2.5.53/sound/isa/opti9xx/opti92x-ad1848.c opti_2-2.5.53/sound/isa/opti9xx/opti92x-ad1848.c
--- opti_1-2.5.53/sound/isa/opti9xx/opti92x-ad1848.c	Mon Dec 30 20:19:47 2002
+++ opti_2-2.5.53/sound/isa/opti9xx/opti92x-ad1848.c	Mon Dec 30 20:48:08 2002
@@ -1674,13 +1674,14 @@
 		if ((err = snd_opti9xx_init(chip, i)) < 0)
 			return err;
 
-		if (check_region(chip->mc_base, chip->mc_base_size))
+		if ((chip->res_mc_base = request_region(chip->mc_base, chip->mc_base_size, "opti9xx_detect")) == NULL)
 			continue;
 
 		value = snd_opti9xx_read(chip, OPTi9XX_MC_REG(1));
 		if ((value != 0xff) && (value != inb(chip->mc_base + 1)))
 			if (value == snd_opti9xx_read(chip, OPTi9XX_MC_REG(1)))
 				return 1;
+		release_region(chip->mc_base, chip->mc_base_size);
 	}
 #else	/* OPTi93X */
 	for (i = OPTi9XX_HW_82C931; i >= OPTi9XX_HW_82C930; i--) {
@@ -1690,7 +1691,7 @@
 		if ((err = snd_opti9xx_init(chip, i)) < 0)
 			return err;
 
-		if (check_region(chip->mc_base, chip->mc_base_size))
+		if ((chip->res_mc_base = request_region(chip->mc_base, chip->mc_base_size, "opti93x_detect")) == NULL)
 			continue;
 
 		spin_lock_irqsave(&chip->lock, flags);
@@ -1703,6 +1704,7 @@
 		snd_opti9xx_write(chip, OPTi9XX_MC_REG(7), 0xff - value);
 		if (snd_opti9xx_read(chip, OPTi9XX_MC_REG(7)) == 0xff - value)
 			return 1;
+		release_region(chip->mc_base, chip->mc_base_size);
 	}
 #endif	/* OPTi93X */
 
@@ -1993,6 +1995,12 @@
 		}
 		if (hw <= OPTi9XX_HW_82C930)
 			chip->mc_base -= 0x80;
+
+		if ((chip->res_mc_base = request_region(chip->mc_base, chip->mc_base_size, "OPTi9xx MC")) == NULL) {
+			snd_card_free(card);
+			return -ENOMEM;
+		}
+
 	} else {
 #endif	/* __ISAPNP__ */
 		if ((error = snd_card_opti9xx_detect(card, chip)) < 0) {
@@ -2002,11 +2010,6 @@
 #ifdef __ISAPNP__
 	}
 #endif	/* __ISAPNP__ */
-
-	if ((chip->res_mc_base = request_region(chip->mc_base, chip->mc_base_size, "OPTi9xx MC")) == NULL) {
-		snd_card_free(card);
-		return -ENOMEM;
-	}
 
 #ifdef __ISAPNP__
 	if (!isapnp) {

Re: [patch, 2.5] opti92x-ad1848 one check_region fixup

[Rusty Russell]

In message <Pine.LNX.4.44.0212302222530.30703-100000@tuxedo.abo.fi> you write:
> Initializes the variables (the chip->xxx stuff) before calling 
> anything else. snd_legacy_find_free_ioport() uses request_region now, 
> so remember to release regions in the private freeing routine 
> snd_card_opti9xx_free().

The patch looks good, but the Trivial Patch Monkey (ook ook!) doesn't
handle chains of patches which depend on each other 8(

So I've only grabbed the first one...
Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Re: [patch, 2.5] opti92x-ad1848 one check_region fixup

[Marcus Alanen]

On Wed, 1 Jan 2003, Rusty Russell wrote:

> The patch looks good, but the Trivial Patch Monkey (ook ook!) doesn't
> handle chains of patches which depend on each other 8(
> 
> So I've only grabbed the first one...

No problem, we've got all the time in the world. ;) I'll resend the 
missing pieces as kernel versions come out.

Happy New Year

Marcus

Re: [patch, 2.5] opti92x-ad1848 one check_region fixup

[Jaroslav Kysela]

On Mon, 30 Dec 2002, Marcus Alanen wrote:

> Initializes the variables (the chip->xxx stuff) before calling 
> anything else. snd_legacy_find_free_ioport() uses request_region now, 
> so remember to release regions in the private freeing routine 
> snd_card_opti9xx_free().
> 
> Note how I changed it to return SNDRV_AUTO_PORT instead of -1, I'm 
> not sure if they are guaranteed to be the same, so I changed it 
> instead explicitely.
> 
> No other snd_legacy_find_free_ioport users in this file or elsewhere 
> in the kernel.

Your patch is bad. Lowlevel drivers allocate the hardware resources (see
snd_cs4231_create() or snd_ad1848_create() code), but these functions will
fail, because you allocate resources in the top-level code. I think that 
it will be sufficient to replace check_region call with request_region and 
release_resource. The collision frame is so small and the code returns 
with an error code when a problem occurs.

						Jaroslav

-----
Jaroslav Kysela <perex@suse.cz>
Linux Kernel Sound Maintainer
ALSA Project, SuSE Labs

Re: [patch, 2.5] opti92x-ad1848 one check_region fixup

[Marcus Alanen]

On Wed, 1 Jan 2003, Jaroslav Kysela wrote:

> Your patch is bad. Lowlevel drivers allocate the hardware resources (see
> snd_cs4231_create() or snd_ad1848_create() code), but these functions will
> fail, because you allocate resources in the top-level code. I think that 

ok, true. Rusty, I think you haven't included these opti_* patches 
since they depend on the 
"[patch, 2.5] move snd_legacy_find_free_ioport to opti92x-ad1848.c"
patch; just drop the opti_* stuff if you have.

> it will be sufficient to replace check_region call with request_region and 
> release_resource.

This is exactly what check_region does already :), so there is no 
point in changing it like that.

Marcus