From owner-freebsd-arch@FreeBSD.ORG Fri Jan 6 18:27:58 2012 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id B3D0E106566C for ; Fri, 6 Jan 2012 18:27:58 +0000 (UTC) (envelope-from marius@alchemy.franken.de) Received: from alchemy.franken.de (alchemy.franken.de [194.94.249.214]) by mx1.freebsd.org (Postfix) with ESMTP id 1B7E28FC08 for ; Fri, 6 Jan 2012 18:27:57 +0000 (UTC) Received: from alchemy.franken.de (localhost [127.0.0.1]) by alchemy.franken.de (8.14.4/8.14.4/ALCHEMY.FRANKEN.DE) with ESMTP id q06IRuIm012805; Fri, 6 Jan 2012 19:27:56 +0100 (CET) (envelope-from marius@alchemy.franken.de) Received: (from marius@localhost) by alchemy.franken.de (8.14.4/8.14.4/Submit) id q06IRuNh012804; Fri, 6 Jan 2012 19:27:56 +0100 (CET) (envelope-from marius) Date: Fri, 6 Jan 2012 19:27:56 +0100 From: Marius Strobl To: Stefan Bethke Message-ID: <20120106182756.GA88161@alchemy.franken.de> References: <8D025847-4BE4-4B2C-87D7-97E72CC9D325@lassitu.de> <20120104215930.GM90831@alchemy.franken.de> <47ABA638-7E08-4350-A03C-3D4A23BF2D7E@lassitu.de> <1763C3FF-1EA0-4DC0-891D-63816EBF4A04@lassitu.de> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1763C3FF-1EA0-4DC0-891D-63816EBF4A04@lassitu.de> User-Agent: Mutt/1.4.2.3i Cc: freebsd-arch@freebsd.org Subject: Re: Extending sys/dev/mii X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 06 Jan 2012 18:27:58 -0000 On Fri, Jan 06, 2012 at 01:57:06PM +0100, Stefan Bethke wrote: > Am 05.01.2012 um 21:52 schrieb Stefan Bethke: > > > 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. > >> > >> 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. > > Here's a patch that causes zero rototilling, if I'm not mistaken. > > The patch implements the split between the MDIO access and notifications posted to the ethernet interface device that has the MII that needs to be adjusted in accordance with the PHY autonegotiation results. I've added a field to the ivars struct and not the softc, because the softc is included by many network drivers, while the ivars are private to mii.c For this reason, I believe this change is API and ABI compatible, and likely can be MFCed. (I believe MFCing is not high on the priority list because many other parts in sys/mips would need to be MFCed first for all the Atheros platforms to become fully usable, but Adrian can correct me.) > By calling an newbus method on a device that is not the parent this patch hacks around how newbus is intended to work. I also still don't see why for the scenarios you describe you can't simply use miibus(4) as-is in a clean way. For the following scenario: fooeth0 | mdiobus0 | \ miibus0 (ethswitch0 or whatever) | foophy0 In mdiobus(4) you'd simply do: DEVMETHOD(miibus_statchg, mdiobus_statchg), <...> static void mdiobus_statchg(device_t dev) { parent = device_get_parent(dev); MIIBUS_STATCHG(device_get_parent(dev)); } in order to bubble up the miibus_statchg-method call from miibus0 to fooeth0. Likewise for the miibus_{linkchg,mediainit,readreg,writereg}. This isn't a work-around but how newbus works, just see the various bus drivers in the tree. I agree that if you'd want to do something like the following in order to "solve" the problem with arge(4): nexus0 / \ arge0 arge1 | mdiobus0 / \ miibus0 miibus1 | | foophy0 foophy1 or even: nexus0 / | \ arge0 arge1 mdio0 / \ miibus0 miibus1 | | foophy0 foophy1 then newbus is inconvenient here as it's just not how newbus is designed. But then calling the miibus_statchg-method etc on the Ethernet driver device also isn't your only problem as there's no newbus way to connect miibus0 with arge0 in the upper or arge0/arge1 with mdio0 in the lower diagram. Such architectures are just one big hack in the newbus point of view as you'd need to sidestep newbus with something like your patch. That's why I proposed the model that puc(4), scc(4) etc are following to solve this in a clean way, which for arge(4) would look like: nexus0 | miimux0 / \ arge0 arge1 | | ethswitch0 | | | miibus0 miibus1 | | foophy0 foophy1 Then you once again can bubble up things just fine. Basically look at scc(4) how to implement this, i.e. miimux(4) (or whatever you'd like to call it) would have a core, various bus-frontends (for nexus(4), pci(4) etc) and MAC-specific device parts for AR71xx etc. Then the probe routine of the nexus-frontend of miimux(4) would need claim the devices for arge0/1 in this setup (you'd probably need to provide a way to easily hard-code or specify this via hints as I can't imagine a way to auto-detect that) by returning a higheR probe value than arge(4) does. Apart from that it just needs to pass-through the bus resources to arge(4) and implement miibus_{readreg,writereg}-methods for the shared AR71xx MDIO master. In turn arge(4) would have two bus-frontends, one for nexus(4) (for the "normal" case) and one for miimux(4). In case it attaches to miimux(4) it then would just bubble up miibus_{readreg,writereg}- calls but terminate miibus_{linkchg,mediainit,statchg)- calls locally. Besides doing whatever it needs to do to configure the switch, ethswitch(4) would also just always bubble up the miibus_if.m methods. Besides being the clean way to do this from the newbus point of view this architecture also needs zero changes to miibus(4). Marius