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

Pyun YongHyeon pyunyh at gmail.com
Mon Jul 3 08:28:16 UTC 2006


On Mon, Jul 03, 2006 at 12:44:04AM -0600, M. Warner Losh wrote:
 > 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.
 > 

Yes I checked NetBSD/OpenBSD mii code. But their probe routine(xxxmatch)
just return positive vlaue when there are alternative drivers. However
using a magic constant in their probe routine made me hard to understand
its meaning. I don't see any reason using a positive value except for
synching drivers with NetBSD.

-- 
Regards,
Pyun YongHyeon


More information about the cvs-all mailing list