Date: Thu, 5 Jan 2012 21:52:38 +0100 From: Stefan Bethke <stb@lassitu.de> To: Marius Strobl <marius@alchemy.franken.de> Cc: freebsd-arch@freebsd.org Subject: Re: Extending sys/dev/mii Message-ID: <47ABA638-7E08-4350-A03C-3D4A23BF2D7E@lassitu.de> In-Reply-To: <20120104215930.GM90831@alchemy.franken.de> References: <8D025847-4BE4-4B2C-87D7-97E72CC9D325@lassitu.de> <20120104215930.GM90831@alchemy.franken.de>
next in thread | previous in thread | raw e-mail | index | archive | help
Thank you for your extensive reply, that really helps my understanding = of the code! Am 04.01.2012 um 22:59 schrieb Marius Strobl: > On Wed, Jan 04, 2012 at 05:03:41PM +0100, Stefan Bethke wrote: >> As discussed recently, ray@, adrian@ and myself are trying to get a = framework and utility into the tree that allows the use and = configuration of ethernet switch chips. The switch controllers we've = looked at so far share a number of features, in particular they use = 802.3 MII, MDIO and PHYs to implement and configure the ports they = offer. In addition to being a switch, some of them also offer one of = the built-in PHYs to the ethernet controller as a classical PHY. >>=20 >> Since the switch is already using MDIO and PHYs, it seems sensible to = reuse the existig sys/dev/mii code. >=20 > The intention of miibus(4) was to factor out the common code for > handling PHYs conforming to IEEE 802.3, specifically chapter 22. > Before Ethernet drivers for controllers that have an MII interface > duplicated that code over and over again. > The problem with switches is that even if they ave an MII management > interface they often don't comply with IEEE 802.3 (also note that > chapter 22 right at the beginning says that the MII is for connecting > MACs with PHYs) in that they don't implement the basic registers and/ > or the ID registers and/or implement something entirely different at > the same addresses. Also a switch with an MII isn't necessarily > connected to a MAC in he first place, f.e. you could simply drive it > via bitbanging using GPIOs of a microcontroller. All that stuff makes > it rather hard to support these with a generic framework that's > laid out around IEEE 802.3. That's also why I think that miibus(4) > isn't the right vehicle to support all these kinds of switches even > if they have an MII interface. If the idea is to only support those > via miibus(4) that actually hang off of a MAC and act like a IEEE > 802.3 PHY I'm fine with that though. The reason I'm even looking at miibus is to avoid code duplication. I = agree that miibus is not the right driver for switches that do not = implement the .3 PHY register set. *However*, the switch chips that = we're looking at (all part of shipping consumer products that I would = love to be able to run FreeBSD on) implement PHYs with the proper = register set. These PHYs can successfully be driven by ukphy. The hard = part: on some switch models, the MDIO these PHYs are on are not the MDIO = the MII is on. I've drawn three diagrams = (http://wiki.freebsd.org/StefanBethke/EtherSwitch) to illustrate two = existing hardware setups that cannot be supported by miibus with either = changes to miibus (AFAIKT). If you feel that changing miibus to allow this split between MDIO and = MII is not advisable, I can create workarounds, but I'd much rather = avoid writing workarounds. >> However, the current code assumes a simple model where the ethernet = controller has one MAC and an MDIO master, and the PHYs are attached to = these two busses. In addition, the code assumes that all attached child = drivers of an miibus will always be PHY drivers (using custom dispatch = table, specific ivars, etc.) >=20 > I'd say that description is to strict. Currently the only real > "limiting" factor is that miibus(4) attaches and adds ifmedia and > while that only partly makes sense for a switch when it's connected > to a MAC it completely doesn't if there's no MAC. But as far as > miibus(4) goes its parent doesn't necessarily need to be a MAC > driver and also while it's children need to have a mii_softc or > at least a softc that begins with a struct mii_softc for the > most part you could also just ignore that miibus(4) assigns instance > variables to it and a child also doesn't necessarily need to do > anything in the mii_phy_funcs that are part of the mii_softc. > A child of the miibus(4) could also just sit there and twiddle some > registers of something that isn't a PHY but then the question is > why you'd want to handle the whole thing via miibus(4) in the first > place =85 Wouldn't that be rather brittle? miibus has a defined interface (and = with your explanations, I believe I now understand it), so poking around = in the innards and forcing parts into the bus that don't really belong = there feels wrong. >> The current miibus code assumes that it is attached to the ethernet = driver, and will call MIIBUS_STATCHG on its parent to inform it of PHY = link changes. >=20 > That's not correct. It's true that miibus(4) calls the statchg method > of its parent but that doesn't necessarily need to be an Ethernet > driver. It could as well be mdiobus(4) which then bubbles up the > request to the Ethernet driver (if there is one). Actually that's > one of the benefits of newbus. In the past there was a requirement > that the parent had the ifnet pointer at the beginning of it's > softc but I fixed that with the introduction of mii_attach() so > you now can supply a ifnet pointer from anywhere higher in the=20 > hierarchy you want. The problem with this is that the miibus instance might not be a = (transitive) child of the ethernet driver that has the MII that needs to = be adjusted to the new PHY settings. And since the method does not = provide any parameters about which phy or miibus did issue the method, = or which ifp it applies to, bubbling it up won't work (that the scenario = where the PHY for arge0 is connected to the switch's MDIO, which is = attached to arge1's MDIO). >> Since the parent will now be the mdiobus, miibus needs effectively = two attachments, one to the provider of the MDIO access, the other for = the ethernet interface. I propose to associate the ethernet interface = by a modified mii_attach() function that takes a device_t (of the = ethernet driver) instead of the two callback function pointers. >=20 > Again I see no technical reason for such a change. While it would > be nice to only need to pass one pointer around instead of two this > IMO doesn't outweight the churn throughout all MAC drivers and the > ABI breakage this would cause, which means that such a change can't > be MFC'ed, needs #ifdefs on drivers that are maintained as multi- > release versions etc. It could very well be that there actually is > the technical necessity to change the API of mii_attach() in order > to support switch PHYs but so far you failed to make clear what > that would be. See above. [ suggested cleanup of methods and callbacks and history lesson removed = ] >> Despite this long description, I believe that the code changes are = relatively minor. The major issue is the proposed ABI change for the = callback functions, which probably will involve updating all drivers = calling mii_attach(). It might be possible to have the existing = function provide a compatibility wrapper around the new attachment code. >=20 > Please elaborate on why these changes are technically necessary > to implement what you are trying do. Otherwise I prefer to avoid > them given the rototilling they'd cause. Necessary is a strong word. Right now, I'm trying to understand how a = sensible change would even look like, and which combination of glue code = and miibus changes make the most sense. Let me see if I can come up with a prototype patch the next couple of = days, so we don't have to theorize about the changes that might or might = not be necessary. Thanks, Stefan --=20 Stefan Bethke <stb@lassitu.de> Fon +49 151 14070811
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?47ABA638-7E08-4350-A03C-3D4A23BF2D7E>