[PATCH] opl3sa2 isapnp activation fix

[PATCH] opl3sa2 isapnp activation fix

[Gerald Teschl]

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",

Re: [PATCH] opl3sa2 isapnp activation fix

[Zwane Mwaikambo]

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
		

Re: [PATCH] opl3sa2 isapnp activation fix

[Zwane Mwaikambo]

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
		

Re: [PATCH] opl3sa2 isapnp activation fix

[Gerald Teschl]

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

Re: [PATCH] opl3sa2 isapnp activation fix

[Zwane Mwaikambo]

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
		

Re: [PATCH] opl3sa2 isapnp activation fix

[Zwane Mwaikambo]

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
		

Re: [PATCH] opl3sa2 isapnp activation fix

[Gerald Teschl]

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

[PATCH] opl3sa2 isapnp activation fix

[Gerald Teschl]

Now here is the new version of the opl3sa2 activation fix. This new
version now adds the fix as a quirk to isapnp. Zwane Mwaikambo and
I have discussed this very carefully and we now both agree that this
is the best way to fix the problem. The patch is against 2.4.19-pre10
and requires the previous isapnp_dma0.patch.

It now loops over all acceptable dma_resources and only allows dma0
if dma channel 0 is the ONLY value accepted by the card. In
addition, it also fixes the problem that a card would not get
deactivated upon removal of the module.

Gerald

opl3sa2_dma0.patch:
-------------------------
--- linux.orig/drivers/pnp/quirks.c	Thu Jun  6 18:04:43 2002
+++ linux/drivers/pnp/quirks.c	Thu Jun  6 18:07:25 2002
@@ -102,6 +102,28 @@
 	return;
 }
 
+extern int isapnp_allow_dma0;
+static void __init quirk_opl3sax_resources(struct pci_dev *dev)
+{
+	/* This really isn't a device quirk but isapnp core code
+	 * doesn't allow a DMA channel of 0, afflicted card is an
+	 * OPL3Sax where x=4.
+	 */
+	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;
+	}
+	if (max == 1 && isapnp_allow_dma0 == -1) {
+		printk(KERN_INFO "isapnp: opl3sa4 quirk: Allowing dma 0.\n");
+		isapnp_allow_dma0 = 1;
+	}
+	return;
+}
+
 /*
  *  ISAPnP Quirks
  *  Cards or devices that need some tweaking due to broken hardware
@@ -133,6 +155,8 @@
 		quirk_sb16audio_resources },
 	{ ISAPNP_VENDOR('C','T','L'), ISAPNP_DEVICE(0x0045),
 		quirk_sb16audio_resources },
+	{ ISAPNP_VENDOR('Y','M','H'), ISAPNP_DEVICE(0x0021),
+		quirk_opl3sax_resources },
 	{ 0 }
 };
 
--- linux.orig/drivers/sound/opl3sa2.c	Thu Jun  6 18:04:44 2002
+++ linux/drivers/sound/opl3sa2.c	Thu Jun  6 18:07:25 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	   ISA PnP activate fix. (Jun 02 2002)
  *
  */
 
@@ -873,10 +874,11 @@
 	}
 	else {
 		if(dev->activate(dev) < 0) {
-			printk(KERN_WARNING PFX "ISA PnP activate failed\n");
+			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",