Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 29 Sep 2001 18:03:07 -0700
From:      Peter Wemm <peter@wemm.org>
To:        Ian Dowse <iedowse@FreeBSD.org>, cvs-committers@FreeBSD.org, cvs-all@FreeBSD.org
Subject:   Re: cvs commit: src/sys/i386/conf NOTES src/sys/conf options src/sys/dev/ed if_ed.c if_ed_pccard.c if_edvar.h 
Message-ID:  <20010930010307.88FB7380A@overcee.netplex.com.au>
In-Reply-To: <20010929235909.E6894380A@overcee.netplex.com.au> 

next in thread | previous in thread | raw e-mail | index | archive | help
Peter Wemm wrote:
> Ian Dowse wrote:
> > iedowse     2001/09/29 15:32:03 PDT
> > 
> >   Modified files:
> >     sys/i386/conf        NOTES 
> >     sys/conf             options 
> >     sys/dev/ed           if_ed.c if_ed_pccard.c if_edvar.h 
> >   Log:
> >   Add an option ED_NO_MIIBUS, which causes the `ed' driver to be
> >   built without support for miibus PHYs. Most ed cards don't need
> >   miibus support, so it's useful to be able to avoid the bloat of
> >   all the mii devices for small fixed-purpose kernels.
> 
> This is actually a bug in miibus.  There should not be references into the
> bowels of the miibus code.  ed should be able to call the methods when
> required, and that is the only time it would depend on it.  And it would be
> able to fail at runtime, not compile time.  Alternatively you could kldload
> the miibus code and it would automagically work.

As a specific example, consider:

static void
ed_tick(arg)
        void *arg;
{ 
        struct ed_softc *sc = arg;
        struct mii_data *mii;
        int s;
 
        if (sc->gone) {
                callout_handle_init(&sc->tick_ch);
                return;
        }
        s = splimp();
        if (sc->miibus != NULL) {
                mii = device_get_softc(sc->miibus);
                mii_tick(mii);
        }
        sc->tick_ch = timeout(ed_tick, sc, hz);
        splx(s);
}

This should be something along the lines of:

static void
ed_tick(arg)
        void *arg;
{ 
        struct ed_softc *sc = arg;
        int s;
 
        if (sc->gone) {
                callout_handle_init(&sc->tick_ch);
                return;
        }
        s = splimp();
        if (sc->miibus != NULL)
                MII_TICK(sc->miibus);
        sc->tick_ch = timeout(ed_tick, sc, hz);
        splx(s);
}

This requires no compile-time or link-time dependencies on the miibus module.

There should be *NO*  mii_xxx() calls in drivers, and no 
"struct mii_data *mii;" and no "mii = device_get_softc(sc->miibus);"
code either.  This is the very layering violation that newbus exists to
prevent.

I am not complaining about your change, just pointing out that the
implemenation needs more work to fit cleanly into the system.

Cheers,
-Peter
--
Peter Wemm - peter@FreeBSD.org; peter@yahoo-inc.com; peter@netplex.com.au
"All of this is for nothing if we don't go to the stars" - JMS/B5


To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe cvs-all" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20010930010307.88FB7380A>