2002-12-30 19:33:14

by Marcus Alanen

[permalink] [raw]
Subject: [patch, 2.5] move snd_legacy_find_free_ioport to opti92x-ad1848.c

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)
{


2002-12-30 20:20:04

by Marcus Alanen

[permalink] [raw]
Subject: [patch, 2.5] opti92x-ad1848 one check_region fixup

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;

2002-12-30 20:49:55

by Marcus Alanen

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

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) {

2002-12-31 23:13:35

by Rusty Russell

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

In message <[email protected]> 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.

2003-01-01 13:41:56

by Marcus Alanen

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

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

2003-01-01 16:30:56

by Jaroslav Kysela

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

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 <[email protected]>
Linux Kernel Sound Maintainer
ALSA Project, SuSE Labs

2003-01-01 17:21:57

by Marcus Alanen

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

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