Bug 24854

Summary: NEWCARD support for Aironet driver an(4)
Product: Base System Reporter: gabriel <gabriel>
Component: kernAssignee: Warner Losh <imp>
Status: Closed FIXED    
Severity: Affects Only Me    
Priority: Normal    
Version: 5.0-CURRENT   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
file.diff none

Description gabriel 2001-02-04 22:50:00 UTC
The Aironet driver is somewhat broken under OLDCARD on recent -current 
(detaching the card causes a panic) but I feel it's better to have it 
correctly supported under NEWCARD right now, as most of the other wireless 
drivers have been already converted (wi(4) and ray(4))

Fix: I'm submitting a patch, very similar to the one made to wi(4). I'm using it 
right now to send this PR so it works OK most of the time. I'm still getting
 panics after I reinsert the card (removal works fine), but I'm not sure if 
that's due to some NEWCARD brokeness.

Verbose card insert messages are available here:
http://devils.maquina.com/~gabriel/boot.an

The patch is at:
http://devils.maquina.com/~gabriel/patch.an
 
But I will also include it here for examination:

How-To-Repeat: Use the Aironet card in any NEWCARD system
Comment 1 the+xp 2001-08-17 09:02:07 UTC
Curious as to why the fix hasn't yet been commited to -current, as it worked
fine for me, and simply represents an extension of what was done for the wi
driver.
Comment 2 the 2001-08-17 21:03:55 UTC
For whatever reason, the calls to pccard_compat_probe|attach
don't succeed in detecting my Aironet 350 series card.

However, hacking if_an_pccard.c in the following way to
change DEVMETHOD(device_[probe|attach] from 'pccard_compat' to 
'an_pccard' works for me:

--- 82,103 ----
  /*
   * Support for PCMCIA cards.
   */
+ static int  an_pccard_match(device_t);
  static int  an_pccard_probe(device_t);
  static int  an_pccard_attach(device_t);
  static int  an_pccard_detach(device_t);

  static device_method_t an_pccard_methods[] = {
        /* Device interface */
!       DEVMETHOD(device_probe,         an_pccard_probe),
!       DEVMETHOD(device_attach,        an_pccard_attach),
        DEVMETHOD(device_detach,        an_pccard_detach),
        DEVMETHOD(device_shutdown,      an_shutdown),

+       /* Card interface */
+       DEVMETHOD(card_compat_match,    an_pccard_match),
+       DEVMETHOD(card_compat_probe,    an_pccard_probe),
+       DEVMETHOD(card_compat_attach,   an_pccard_attach),
        { 0, 0 }
  };

The updated patch is availabe at:

http://www.bestII.com/freebsd/patch.an.new

Take care,

--Sam
Comment 3 Warner Losh freebsd_committer freebsd_triage 2001-08-18 18:42:42 UTC
Responsible Changed
From-To: freebsd-bugs->imp

I'll look into this.
Comment 4 Jonathan Chen freebsd_committer freebsd_triage 2001-08-19 14:14:52 UTC
[summary of past events for the benefit of gnats]
 - original PR filed for Cisco aironet not working under newcard
 - My LMC342 works just fine, and I though this might be a bug in newcard 
   (as opposed to the an driver as suggested in the pr), suggested Sam to 
   try the updated newcard (http://people.freebsd.org/~jon/newcard.diff.3)
 - this patch makes an work under newcard without the proposed changes in 
   the pr.

> I believe I already tried just uncommenting 'optional ata pccard'
> with the -current source, without any luck. 

Sam, I'm not sure what you mean here.

> The first time I tried it (not in X) removal and reinsertion of the 
> Aironet card was fine.
> 
> The second time (while in X), removal was ok, but card reinsertion caused 
> a reboot (no panic)...
> 
> Let me know what you want me to do exactly, in terms of what kind of 
> debugging you need...once I hear back I'll build a debug kernel.

Are you sure there is no panic while in X?  Your X server might have 
futzed with the video card so you might not actually see the panic.  If you 
can try to reproduce this in text mode, then the panic message as well as a 
traceback would be a very good start.

But before you do that, I believe I may have found a fix for a possible 
panic situation.  This panic occurs in witness_destroy as a supervisor read 
page not present error (I presume while trying to dereference lock).  This 
panic can be easily reproduced by running dhclient an0, removing and 
reinserting the card, then killing dhclient.  The fix appears to be 
shockingly simple:

diff -u -r1.8 if_an_pccard.c
--- sys/dev/an/if_an_pccard.c	2001/05/26 09:26:58	1.8
+++ sys/dev/an/if_an_pccard.c	2001/08/19 12:57:14
@@ -118,6 +118,7 @@
 	sc->an_gone = 1;
 	bus_teardown_intr(dev, sc->irq_res, sc->irq_handle);
 	an_release_resources(dev);
+	mtx_destroy(&sc->an_mtx);
 	return (0);
 }
 

Here's where I require some guidance from someone in the know.  While this 
patch appears to avoid the problem on my system, does it in fact work?  How 
does the panic occur and how does this fix the problem, if indeed it does?
I'm quite clueless as to the implementation of mutex locking on freebsd, 
and I suppose it's just plain luck that the first thing I tried turned out 
to have done the trick...

-Jon
Comment 5 Sam Habash 2001-08-19 18:39:55 UTC
On Sun, Aug 19, 2001 at 09:14:52AM -0400, Jonathan Chen wrote:
> 
> [summary of past events for the benefit of gnats]
>  - original PR filed for Cisco aironet not working under newcard
>  - My LMC342 works just fine, and I though this might be a bug in newcard 
>    (as opposed to the an driver as suggested in the pr), suggested Sam to 
>    try the updated newcard (http://people.freebsd.org/~jon/newcard.diff.3)
>  - this patch makes an work under newcard without the proposed changes in 
>    the pr.
> 
> > I believe I already tried just uncommenting 'optional ata pccard'
> > with the -current source, without any luck. 
> 
> Sam, I'm not sure what you mean here.

What I mean here is that an unpatched -current (as of mid-August)
was not resulting in detection of my LMC352.  In the current sources, 
the following in src/sys/conf/files has been commented out:

#dev/an/if_an_pccard.c   optional an pccard

Commenting this out and recompiling did NOT result in a successful
attach, at least when I tested this in early August.  

I'm testing again by backing out of your patch and recompiling, and 
it turns out that it -does- work now, most likely this was operator 
error in not uncommenting the the conf/files entry, since I don't 
see any likely commits that would have changed anything...sigh,
sorry for the waste of time.

http://www.freebsd.org/cgi/cvsweb.cgi/src/sys/conf/files.diff?r1=1.483&r2=1.484&f=h

documents when the line was commented out...it's been that way for
a while...

> > The first time I tried it (not in X) removal and reinsertion of the 
> > Aironet card was fine.
> > 
> > The second time (while in X), removal was ok, but card reinsertion caused 
> > a reboot (no panic)...
> > 
> > Let me know what you want me to do exactly, in terms of what kind of 
> > debugging you need...once I hear back I'll build a debug kernel.
> 
> Are you sure there is no panic while in X?  Your X server might have 
> futzed with the video card so you might not actually see the panic.  If you 
> can try to reproduce this in text mode, then the panic message as well as a 
> traceback would be a very good start.

-nod-

> But before you do that, I believe I may have found a fix for a possible 
> panic situation.  This panic occurs in witness_destroy as a supervisor read 
> page not present error (I presume while trying to dereference lock).  This 
> panic can be easily reproduced by running dhclient an0, removing and 
> reinserting the card, then killing dhclient.  The fix appears to be 
> shockingly simple:

I'll see if I can reproduce the problem in CLI mode with your patch
to if_an_pccard.c

> 
> diff -u -r1.8 if_an_pccard.c
> --- sys/dev/an/if_an_pccard.c	2001/05/26 09:26:58	1.8
> +++ sys/dev/an/if_an_pccard.c	2001/08/19 12:57:14
> @@ -118,6 +118,7 @@
>  	sc->an_gone = 1;
>  	bus_teardown_intr(dev, sc->irq_res, sc->irq_handle);
>  	an_release_resources(dev);
> +	mtx_destroy(&sc->an_mtx);
>  	return (0);
>  }

Take care,

--Sam
Comment 6 Warner Losh freebsd_committer freebsd_triage 2001-11-15 06:19:04 UTC
State Changed
From-To: open->closed

Fixed.