2002-06-04 15:58:29

by Gerald Teschl

[permalink] [raw]
Subject: [PATCH] opl3sa2 isapnp activation fix

Hi,

this is the final version of my patch which fixes the isapnp activation
of opl3sa2 based
cards which require dma=0 as discussed with Zwane Mwaikambo.

Gerald

--- linux-2.4.18-4/drivers/sound/opl3sa2.c.orig Thu May 2 23:36:45 2002
+++ linux-2.4.18-4/drivers/sound/opl3sa2.c Tue Jun 4 16:09:50 2002
@@ -57,6 +57,7 @@
* (Jan 7, 2001)
* Zwane Mwaikambo Added PM support. (Dec 4 2001)
* Zwane Mwaikambo Code, data structure cleanups. (Feb 15 2002)
+ * Gerald Teschl Fixed ISA PnP activate. (Jun 02 2002)
*
*/

@@ -869,10 +870,24 @@
}
else {
if(dev->activate(dev) < 0) {
- printk(KERN_WARNING PFX "ISA PnP activate failed\n");
- opl3sa2_state[card].activated = 0;
- return -ENODEV;
+ /*
+ * isapnp.c disallows dma=0 but some opl3sa2 cards need it.
+ * So we set dma by hand and try again
+ */
+ if (dma < 0 || dma > 7)
+ dma= 0;
+ if (dma2 < 0 || dma2 >7)
+ dma2= 1;
+ isapnp_resource_change(&dev->dma_resource[0], dma, 1);
+ isapnp_resource_change(&dev->dma_resource[1], dma2, 1);
}
+ if(!dev->active)
+ if (dev->activate(dev) < 0) {
+ printk(KERN_WARNING PFX "ISA PnP activate failed.\n");
+ opl3sa2_state[card].activated = 0;
+ return -ENODEV;
+ }
+ opl3sa2_state[card].activated = 1;

printk(KERN_DEBUG
PFX "Activated ISA PnP card %d (active=%d)\n",


2002-06-04 16:11:07

by Zwane Mwaikambo

[permalink] [raw]
Subject: Re: [PATCH] opl3sa2 isapnp activation fix

On Tue, 4 Jun 2002, Gerald Teschl wrote:

> --- linux-2.4.18-4/drivers/sound/opl3sa2.c.orig Thu May 2 23:36:45 2002
> +++ linux-2.4.18-4/drivers/sound/opl3sa2.c Tue Jun 4 16:09:50 2002
> @@ -57,6 +57,7 @@
> * (Jan 7, 2001)
> * Zwane Mwaikambo Added PM support. (Dec 4 2001)
> * Zwane Mwaikambo Code, data structure cleanups. (Feb 15 2002)
> + * Gerald Teschl Fixed ISA PnP activate. (Jun 02 2002)
> *
> */
>
> @@ -869,10 +870,24 @@
> }
> else {
> if(dev->activate(dev) < 0) {
> - printk(KERN_WARNING PFX "ISA PnP activate failed\n");
> - opl3sa2_state[card].activated = 0;
> - return -ENODEV;
> + /*
> + * isapnp.c disallows dma=0 but some opl3sa2 cards need it.
> + * So we set dma by hand and try again
> + */
> + if (dma < 0 || dma > 7)
> + dma= 0;
> + if (dma2 < 0 || dma2 >7)
> + dma2= 1;

Oops, that won't work on isapnp since dma = dma2 = -1 at this stage, how
about;

if ((dma != -1) && (dma2 != -1)) frob();

you shouldn't hard set 0,1

> + isapnp_resource_change(&dev->dma_resource[0], dma, 1);
> + isapnp_resource_change(&dev->dma_resource[1], dma2, 1);
> }
> + if(!dev->active)
> + if (dev->activate(dev) < 0) {
> + printk(KERN_WARNING PFX "ISA PnP activate failed.\n");
> + opl3sa2_state[card].activated = 0;
> + return -ENODEV;
> + }
> + opl3sa2_state[card].activated = 1;
>
> printk(KERN_DEBUG
> PFX "Activated ISA PnP card %d (active=%d)\n",

The rest looks ok.

Cheers,
Zwane
--
http://function.linuxpower.ca



2002-06-04 16:17:25

by Zwane Mwaikambo

[permalink] [raw]
Subject: Re: [PATCH] opl3sa2 isapnp activation fix

Hi Gerald,

I forgot to mention, Marcelo is the more likely target for this one, but
since he's going for -rc you can either wait for 2.4.20-pre or you can
send it to Alan (who probably has the latest version). 2.5 needs a merge
from 2.4-ac which i'll send later on.

Cheers,
Zwane

--
http://function.linuxpower.ca


2002-06-04 16:24:16

by Zwane Mwaikambo

[permalink] [raw]
Subject: Re: [PATCH] opl3sa2 isapnp activation fix

On Tue, 4 Jun 2002, Zwane Mwaikambo wrote:

> > if(dev->activate(dev) < 0) {
> > - printk(KERN_WARNING PFX "ISA PnP activate failed\n");
> > - opl3sa2_state[card].activated = 0;
> > - return -ENODEV;
> > + /*
> > + * isapnp.c disallows dma=0 but some opl3sa2 cards need it.
> > + * So we set dma by hand and try again
> > + */
> > + if (dma < 0 || dma > 7)
> > + dma= 0;
> > + if (dma2 < 0 || dma2 >7)
> > + dma2= 1;
>
> Oops, that won't work on isapnp since dma = dma2 = -1 at this stage, how
> about;
>
> if ((dma != -1) && (dma2 != -1)) frob();
>
> you shouldn't hard set 0,1

You can move the check earlier on so that you do the first activate with
the supplied dma settings, therefore avoiding the double take. That way
you'd also not add any additional frobbing for the normal case.

--
http://function.linuxpower.ca


2002-06-04 17:35:15

by Gerald Teschl

[permalink] [raw]
Subject: Re: [PATCH] opl3sa2 isapnp activation fix

Zwane Mwaikambo wrote:

>On Tue, 4 Jun 2002, Zwane Mwaikambo wrote:
>
>
>
>>> if(dev->activate(dev) < 0) {
>>>- printk(KERN_WARNING PFX "ISA PnP activate failed\n");
>>>- opl3sa2_state[card].activated = 0;
>>>- return -ENODEV;
>>>+ /*
>>>+ * isapnp.c disallows dma=0 but some opl3sa2 cards need it.
>>>+ * So we set dma by hand and try again
>>>+ */
>>>+ if (dma < 0 || dma > 7)
>>>+ dma= 0;
>>>+ if (dma2 < 0 || dma2 >7)
>>>+ dma2= 1;
>>>
>>>
>>Oops, that won't work on isapnp since dma = dma2 = -1 at this stage, how
>>about;
>>
>>if ((dma != -1) && (dma2 != -1)) frob();
>>
I don't get what you mean? I tested this, if I do "modprobe opl3sa2
dma=1 dma2=3" it will activate
the card with dma 1,3 (according to /proc/isapnp). However, my card will
not work with these values.

>>
>>you shouldn't hard set 0,1
>>
The idea is that I first try to activate the card without assigning
fixed values to dma. If this works, then fine. If not,
use whatever the user wants for dma respectively take 0,1 as default
value. If I do not choose default values, then it
will not work automatically unless the user specifies dma. But if the
user has to specify values, this is not PnP
IMHO.

From your previous message I figured that my patch is fine with you?

Gerald


2002-06-04 18:02:14

by Zwane Mwaikambo

[permalink] [raw]
Subject: Re: [PATCH] opl3sa2 isapnp activation fix

On Tue, 4 Jun 2002, Gerald Teschl wrote:

> >>Oops, that won't work on isapnp since dma = dma2 = -1 at this stage, how
> >>about;
> >>
> >>if ((dma != -1) && (dma2 != -1)) frob();
> >>
> I don't get what you mean? I tested this, if I do "modprobe opl3sa2
> dma=1 dma2=3" it will activate
> the card with dma 1,3 (according to /proc/isapnp). However, my card will
> not work with these values.

How about this, you check if the resource register reads 0 for the DMA
value and you reassign it using the resource registers, that way you skip
on using magic values.

Cheers,
Zwane

--
http://function.linuxpower.ca






2002-06-05 15:06:03

by Gerald Teschl

[permalink] [raw]
Subject: Re: [PATCH] opl3sa2 isapnp activation fix

Zwane Mwaikambo wrote:

>Isn't 0 listed in a BAR register anyway? Can't you read it after a
>prepare?
>
I don't know what you mean by a BAR register. As far as I can tell,
prepare does nothing than
setting all values in the dev struct to zero and the flags to AUTO. So
after the prepare call

dev->dma_resource[i].start = dev->dma_resource[i].end = 0


for i=0,1. So I don't think this can be used for a test.

The current version tests ALL dma resources which the card lists as
acceptable for the first dma.
If there is no acceptable configuration other than dma=0, then I give it
dma=0 (see also below).

>My only objection to your patch is the hardcoded 0 and 1, get rid
>of that and you'll get rid of my nagging ;)
>
What is the differnece between your suggestion

if (a == 0)
b = a;

and my

if (a == 0)
b = 0;

>
>+ struct isapnp_resources *res;
>+ int max;
>+ res = (struct isapnp_resources *)dev->sysdata;
>+ max = res->dma->map;
>+ for (res = res->alt; res; res = res->alt) {
>+ if (res->dma->map > max)
>+ max = res->dma->map;
>+ }
>
>I'm sure you don't have to go to the extent of walking the resource lists.
>
I don't see any other way.

>
>+ if (max == 1) {
>+ /* isapnp.c disallows dma=0 but this card only
>accepts dma=0. */
>+ printk(KERN_DEBUG PFX "Setting dma 0.\n");
>+ isapnp_resource_change(&dev->dma_resource[0], 0,
>1);
>+ }
>
>This is dangerous on some boxes, especially if the card failed detect for
>some reason and we retry with DMA-0
>
As I saied, "max == 1" implies that the card will not accept any other
value. So activation will fail for
sure unless I assign dma=0. The current version is better since the old
version would use dma=0 even
if the activate failed for reasons other than dma settings. Now, dma=0
will NOT be used if there is ANY other
configuration acceptable by the card. If there is an opl3sa2 card in a
box which only works with dma=0,
but where the box does not allow dma=0, this system is already broken in
the first place.

BTW, is this "some BIOS don't allow dma 0" somewhere documented? Is this
a BIOS bug or a "normal" case.
I could not find anything on this other then the comment in isapnp.c.

Moreover, what happens if you use dma=0 on such a box?

case a) The sound card does not work.
case b) The box locks up

if a) It will not work any other way, so it makes no difference.
if b) With my patch the box will lock up upon insertion of the sound module.
Without my patch, the user will eventually assign dma=0 by hand
and the box will lock up.
Again no difference IMHO?

Without my patch this card will never work out of the box. But there
seems to be a real need for this
as far as I can tell from the amount of email I got with respect to the
web page where I explain how
to get linux running on this f$%^ asus laptop.

Thanks for your help to find the right(tm) solution,
Gerald