Extending sys/dev/mii

[Stefan Bethke]

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.
>> 
>> Since the switch is already using MDIO and PHYs, it seems sensible to reuse the existig sys/dev/mii code.
> 
> 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.)
> 
> 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 …

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.
> 
> 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 
> 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.
> 
> 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.
> 
> 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

-- 
Stefan Bethke <stb at lassitu.de>   Fon +49 151 14070811