Re: if_iwn.c patch to support the 5100... (analysis + patch)

[Sverre Froyen <sverre%viewmark.com@localhost>]

On Fri September 18 2009 04:05:19 Brett Lymn wrote:
> On Thu, Sep 17, 2009 at 09:39:20AM -0600, Sverre Froyen wrote:
> > It looks like these errors may be because iwn_init is called a second
> > time before the first call has completed.  It is called via the "if_init"
> > pointer and also through iwn_ioctl.  The call via the ioctl path only
> > happens if the IFF_UP is set.  I notice that, e.g., net/if_ethersubr.c
> > sets this flag before the call to if_init.  While this seems
> > counterintuitive to me it is appears to be the way it's always worked. 
> > The simplest fix is to remove the call to iwn_init from iwn_ioctl:
> 
> Doesn't that have the unfortunate side effect of you then being unable
> to reset the interface by doing an ifconfig down/up?

You are correct.  Calling it a patch was an exaggeration.  I quick test shows 
that ifconfig up/down indeed leaves the interface in a non-working state.  
(Still better than a panic at boot, though.)

In order to serialize access (or really prevent a second simultaneous access) 
is the following the correct approach?

/* Should init_lock be part of softc or malloced in iwn_attach? */
uint init_lock = 0;

static int
iwn_init(...) {
        if (atomic_cas_uint(&init_lock, 0, 1) != 0)
                return 0;
        ...
        if (error) [
                init_lock = 0;
                return error;
        }
        ...
        init_lock = 0;
        return 0;
}

Thanks,
Sverre