Date: Mon, 3 Jul 2006 16:48:53 +0900 From: Pyun YongHyeon <pyunyh@gmail.com> To: "M. Warner Losh" <imp@bsdimp.com> Cc: cvs-src@FreeBSD.org, yongari@FreeBSD.org, src-committers@FreeBSD.org, cvs-all@FreeBSD.org, nate@root.org Subject: Re: 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 Message-ID: <20060703074853.GA64457@cdnetworks.co.kr> In-Reply-To: <20060703.004404.1878036141.imp@bsdimp.com> References: <20060703025355.AA9CA16A62D@hub.freebsd.org> <44A8B572.6030503@root.org> <20060703.004404.1878036141.imp@bsdimp.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Jul 03, 2006 at 12:44:04AM -0600, M. Warner Losh wrote: > In message: <44A8B572.6030503@root.org> > Nate Lawson <nate@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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20060703074853.GA64457>