mii misfeature/bug - mii_statchg

[Matt Thomas]

Many SoC these days have a single MDIO bus, off which multiple PHYs are
connected. Now all these PHYs are not used by one interface, but each
are used by their own interface.

Due to recent changes in newer Freescale QorIQ chips, I decided to split
the mdio bus out of the ethernet and make visible in the device hierarchy.
Here's a recent dmesg:

tsec0 at cpunode0 instance 1 phy 0
mdio0 at tsec0
tsec0: Ethernet address 00:04:9f:01:24:e3
ukphy0 at mdio0 phy 0: Vitesse VSC8244 Quad 10/100/1000BASE-T PHY (OUI 0x00c08f, model 0x002c), rev. 2
ukphy0: 10baseT, 10baseT-FDX, 100baseTX, 100baseTX-FDX, 1000baseT, 1000baseT-FDX, auto
tsec1 at cpunode0 instance 2 mdio 0 phy 1
tsec1: Ethernet address 00:04:9f:01:24:e4
ukphy1 at mdio0 phy 1: Vitesse VSC8244 Quad 10/100/1000BASE-T PHY (OUI 0x00c08f, model 0x002c), rev. 2
ukphy1: 10baseT, 10baseT-FDX, 100baseTX, 100baseTX-FDX, 1000baseT, 1000baseT-FDX, auto
tsec2 at cpunode0 instance 3 mdio 0 phy 2
tsec2: Ethernet address 00:04:9f:01:24:e5
ukphy2 at mdio0 phy 2: Vitesse VSC8244 Quad 10/100/1000BASE-T PHY (OUI 0x00c08f, model 0x002c), rev. 2
ukphy2: 10baseT, 10baseT-FDX, 100baseTX, 100baseTX-FDX, 1000baseT, 1000baseT-FDX, auto

As you can see, each phy is off mdio0 but phy is connected to the
appropriate tsec interface.

The problem comes in that mii_statchg always passes the parent of the
phy (mdio0) but it should pass the device_t of the attached interface.
I think the right (but messy) change is to pass "mii_ifp" to statchg
and change mii_data.mii_statchg to take a struct ifnet * instead of a
device_t. But requires changing a lot of drivers. Instead I added
a mii_ifstatchg which passes a struct ifnet *. The user can decide
which to use.

Index: dev/mii/mii_physubr.c
===================================================================
RCS file: /cvsroot/src/sys/dev/mii/mii_physubr.c,v
retrieving revision 1.73
diff -u -p -r1.73 mii_physubr.c
--- dev/mii/mii_physubr.c 10 Dec 2011 02:46:07 -0000 1.73
+++ dev/mii/mii_physubr.c 21 Jul 2012 20:04:42 -0000
@@ -408,7 +408,10 @@ mii_phy_update(struct mii_softc *sc, int
sc->mii_media_status != mii->mii_media_status ||
cmd == MII_MEDIACHG) {
mii_phy_statusmsg(sc);
- (*mii->mii_statchg)(device_parent(sc->mii_dev));
+ if (mii->mii_ifstatchg != NULL)
+ (*mii->mii_ifstatchg)(mii->mii_ifp);
+ else if (mii->mii_statchg != NULL)
+ (*mii->mii_statchg)(device_parent(sc->mii_dev));
sc->mii_media_active = mii->mii_media_active;
sc->mii_media_status = mii->mii_media_status;
}
Index: dev/mii/miivar.h
===================================================================
RCS file: /cvsroot/src/sys/dev/mii/miivar.h,v
retrieving revision 1.59
diff -u -p -r1.59 miivar.h
--- dev/mii/miivar.h 30 May 2010 17:44:08 -0000 1.59
+++ dev/mii/miivar.h 21 Jul 2012 20:04:42 -0000
@@ -50,6 +50,7 @@ struct mii_softc;
typedef int (*mii_readreg_t)(device_t, int, int);
typedef void (*mii_writereg_t)(device_t, int, int, int);
typedef void (*mii_statchg_t)(device_t);
+typedef void (*mii_ifstatchg_t)(struct ifnet *);

/*
* A network interface driver has one of these structures in its softc.
@@ -82,6 +83,7 @@ struct mii_data {
mii_readreg_t mii_readreg;
mii_writereg_t mii_writereg;
mii_statchg_t mii_statchg;
+ mii_ifstatchg_t mii_ifstatchg;
};
typedef struct mii_data mii_data_t;

Comments?


--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[Darren Reed]

Post by Matt Thomas
...
The problem comes in that mii_statchg always passes the parent of the
phy (mdio0) but it should pass the device_t of the attached interface.
I think the right (but messy) change is to pass "mii_ifp" to statchg
and change mii_data.mii_statchg to take a struct ifnet * instead of a
device_t. But requires changing a lot of drivers. Instead I added
a mii_ifstatchg which passes a struct ifnet *. The user can decide
which to use.

That sounds like you're inviting future bugs by way of having two
competing interfaces, not to mention the "do I implement/use X or
Y or both?"

And since when has changing a lot of drivers stopped NetBSD taking
the high road?

Darren


--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de