cvs commit: src/sys/dev/mii acphy.c amphy.c bmtphy.c brgphy.c ciphy.c e1000phy.c exphy.c inphy.c lxtphy.c mlphy.c nsgphy.c nsphy.c pnaphy.c qsphy.c rgephy.c rlphy.c ruephy.c tdkphy.c tlphy.c ukphy.c xmphy.c

[M. Warner Losh]

In message: <44A8B572.6030503 at root.org>
            Nate Lawson <nate at root.org> writes:
: Pyun YongHyeon wrote:
: > yongari     2006-07-03 02:53:40 UTC
: > 
: >   FreeBSD src repository
: > 
: >   Modified files:
: >     sys/dev/mii          acphy.c amphy.c bmtphy.c brgphy.c ciphy.c 
: >                          e1000phy.c exphy.c inphy.c lxtphy.c 
: >                          mlphy.c nsgphy.c nsphy.c pnaphy.c qsphy.c 
: >                          rgephy.c rlphy.c ruephy.c tdkphy.c 
: >                          tlphy.c ukphy.c xmphy.c 
: >   Log:
: >   Replace hard-coded magic constants to system defined constants
: >   (BUS_PROBE_DEFAULT, BUS_PROBE_GENERIC etc).
: >   There is no functional changes.
: >   
: >   Reviewed by:    oleg, scottl
: 
: Actually, there are functional changes.  Whether those changes are ok or 
: not, I don't know.

They are fine.  There are three functional changes.  You only noticed
one of them.  Since this is an important point, I've replied here.

: > --- src/sys/dev/mii/acphy.c:1.17	Fri Sep 30 19:39:27 2005
: > +++ src/sys/dev/mii/acphy.c	Mon Jul  3 02:53:39 2006
: > @@ -132,7 +132,7 @@
: >  	} else 
: >  		return (ENXIO);
: >  
: > -	return (0);
: > +	return (BUS_PROBE_DEFAULT);
: >  }
: >  
: >  static int
: 
: This means probe() will be called multiple times to allow bidding for 
: the device.  Is that ok for this and other devices?

While generally a good, and subtle, point to me, as far as I can tell
it is ok.  All of these probe routines just lookup things by name,
which is safe to do multiple times.

The second subtle thing to look at, btw, is to make sure that the probe
routine doesn't have any side effects.  Like saving values in softc.
I believe that this is the case here as well.

The third subtle thing is now other probe routines get to bid on the
device.  When 0 is returned, we short-circuit the probe process.  We
don't call the other routines.  When -X is returned, we call them
all, and then call the winning bidder again (which is the first one
above).  In some cases, this can cause other probe routines to run
that wouldn't have before for other devices.  Typically, this isn't a
big deal, but it is something to consider.

We really should have a mii lookup routine ala the pccard ones, but
I've not yet sorted out the mii mess.  Did you know we use a different
bit order than NetBSD, which makes sharing drivers harder?  Or at
least bringing in changes to existing drivers?  Maybe this would make
a good jkh project.

Warner