cvs commit: src/sys/dev/fxp if_fxp.c

[Nate Lawson]

On Fri, 11 Apr 2003, Maxime Henrion wrote:
> Nate Lawson wrote:
> > On Thu, 10 Apr 2003, Maxime Henrion wrote:
> > >   Modified files:
> > >     sys/dev/fxp          if_fxp.c 
> > >   Log:
> > >   - Clean up the fxp_release() and fxp_detach() functions.
> > 
> > There's a version of this in the diff I just posted to current at .
> 
> Feel free to commit it.

I'd like to keep it bundled with the MPSAFE changes so it will have to
wait a little while for more testing.

> > I have been testing my diff by loading and unloading fxp while doing a
> > large transfer and I cannot replicate this.  Are you sure it's not a local
> > problem?  I have never had a deadlock or a crash and loading fxp again
> > always works.
> 
> I have no idea, this is why I was saying it's maybe not fxp(4)'s fault
> and it may indeed be a local problem.

It would help if others gave their insight but if loading/unloading work
for me (and have been for months), it suggests it is local.  I don't have
a problem with making the unmap conditional on the tag (first part of
commit).
 
> > Um, fxp_detach() should not be called for any case where the device isn't
> > alive.  fxp_detach should ONLY be called once attach has succeeded which
> > by definition means the device is alive.  bus_child_present() is the
> > bus-specific method to see that the hardware is actually there;
> > device_is_alive only tells you that the device_t node is present in the
> > tree.  fxp_release may be called in error cases.  Rather than working
> > around your problem this way, please find what is calling fxp_detach when
> > the device is not alive.
> 
> This way of doing things (ie: using device_is_alive()) is an almost
> verbatim copy of what you did in every network driver in sys/pci/.
> Now if it's wrong, feel free to change it, but I guess you'll have
> to change all those drivers too then.

The reason why I did that for the other drivers is that there was not a
separate "release" function which deallocated resources.  Instead, I had a
common routine which did detach and release.  For the error case, you do
need to check if the device was alive before calling *stop() on
potentially non-working hardware.

But for fxp, this is done explicitly -- routines which can only be done on
a "live" device go in fxp_detach() and those that need to be done in both
the error and detach cases go in fxp_release().  Thus the second part of
your commit is unnecessary.

-Nate