Date: Wed, 4 Jan 2012 22:59:30 +0100 From: Marius Strobl <marius@alchemy.franken.de> To: Stefan Bethke <stb@lassitu.de> Cc: freebsd-arch@freebsd.org Subject: Re: Extending sys/dev/mii Message-ID: <20120104215930.GM90831@alchemy.franken.de> In-Reply-To: <8D025847-4BE4-4B2C-87D7-97E72CC9D325@lassitu.de> References: <8D025847-4BE4-4B2C-87D7-97E72CC9D325@lassitu.de>
next in thread | previous in thread | raw e-mail | index | archive | help
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.
> 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 ...
>
> I'd like to extend miibus in such a way that the one-to-one mapping between MDIO and MII is broken up. For that, I propose to add a new bus driver "mdiobus" (with appropriate resource management) that uses methods similar to miibus_if.m readreg and writereg to access an ethernet controllers' MDIO master. miibus then attaches to it as a child, claims one or more PHY addresses and attaches PHYs to itself (as currently implemented).
>
> This allows our new switch drivers to attach to the mdiobus as children and claim appropriate PHY addresses as resources, as well.
This generally sounds fine so far.
>
> 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.
> 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.
> At the same time, I'd like to unravel the use of callbacks in miibus and in the PHY drivers. On the one hand the miibus_if.m has three callbacks (statchg, linkchg, mediainit), on the other hand the bus keeps three function pointers (mii_data.mii_readreg, mii_writereg, mii_statchg) that it never uses(?), plus two others (miibus_ivars.ifmedia_upd and ifmedia_sts) that are regularly called to provide link updates to the interface. I would be interested to learn why these are spread out like this (hysterical raisins?), and why they can't just be handled in the usual bus method manner. (Documentation wouldn't hurt either :-)
Just read the comments and the VCS history; mii{,bus}(4) origins
in BSD/OS and was reimplemented in NetBSD using a compatible API.
>From there it was ported/copied to OpenBSD and also ported to
FreeBSD. The FreeBSD implementation is a compromise between
integrating it with newbus and maintaining API-compatibility
with NetBSD (to-date the FreeBSD miibus(4) actually is still
highly API-compatible with NetBSD; if you use the helper
functions FreeBSD has grown since then all it really takes to
port a PHY driver from NetBSD is to replace some headers and to
adjust the arguments of the probe and attach methods). The
newbus integration without doubts could have been done a bit
better in some respects though.
The mii_readreg, mii_writereg and mii_statchg pointers of struct
mii_data you mention are what NetBSD and OpenBSD use instead of
the newbus interface. I'm aware that they are unused in FreeBSD
and they probably were forgotten to be removed when the code
was ported over. Though as they already were there I decided to
keep them as placeholders in case we wanted to MFC an otherwise
ABI-breaking change extending struct mii_data back. It might make
sense to rename them in order to make it clear that they only
serve as placeholders.
The ifmedia_upd and ifmedia_sts pointers could be implemented
as newbus methods as well but given that these are passed to
ifmedia_init(9) using pointers passed via instance variables
probably was the more straight forward approach given that
you don't really need to support a hierarchy here.
>
> 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.
>
> There's one issue that I don't have a proposal for yet: in one platform (AR7241), we have PHY4 of the SoC talking via MII to arge0's MAC, while it is being controlled via the switch controller's MDIO master, and the switch controller being attached to arge1's MDIO. If we want to attach an miibus for PHY4, we'd have to defer attachment of arge0 until arge1 has been probed and can provide the MDIO attachment (and transitively the switch and it's mdio). Note that we also have boards without a switch, but the two PHYs still being attached to only a single MDIO. One possible way would be for the MDIO driver to be separate from the ethernet driver, so that the normal newbus dependency resolution can be used to ensure that mdio1 is attached before arge0 is probed. For the time being, I've worked around this through hackery in if_arge.c.
>
What I proposed here to properly deal with this case is to
multiplex the MII sharing above the MAC drivers, which then
also is independent of the probe order:
http://lists.freebsd.org/pipermail/freebsd-net/2011-December/030646.html
With your latter approach it's unclear to me how you would
link mdio1 and arge0, which would be siblings on the same
bus if I get you right, i.e. either you attach miibus(4) to
mdio(4) and then would need to pass the status and update
handlers of arge0 to mdio1 or you attach miibus(4) to
arge1 and then would need to somehow access the MII readreg
and writereg methods of mdio1 there.
Marius
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120104215930.GM90831>