MII bus API for PHY devices

Replying this to the netdev list for their perusal.

First, I'm flattered that you've based this on the gianfar phy code, which I stole shamelessly from Benh's code, but Benh changed his code, and so I stole once more (the sungem_phy code he mentioned). The current 2.6.9 gianfar driver has a completely different PHY infrastructure. One which is a better model, I think, for abstraction than my previous code.

Which brings me to item #2: I have taken the code from the previous patch, and combined it with my newer PHY code, which should ease the way for someone to add some of the WOL features, and such. I have compiled, and tested this code, and it works with the gianfar driver.

However, I've got a few questions for the network device community, on how best to finish this off.

The primary issue is how to classify the MII bus. Should it be a proper bus, fitting within the new driver model? This seems like the proper method to me, since the MII bus is, in fact, a bus. But it's a bit of a strange bus. The bus is attached at two ends. One end has some number of PHY devices, and the corresponding drivers for them. The other end has some number of ethernet controllers, and their drivers.

So we have 3 implementation decisions which are affected by this:

1) How should we pass initialization information from the system to the bus. Information like which irq to use for each PHY, and what the address space for the bus's controls is. I would like to enforce encapsulation so that the ethernet drivers don't need to know this information, or pass it to the bus.

2) How should we reflect the dependency of the ethernet driver on the mii bus driver?

3) How should we bind ethernet drivers to PHY drivers?

Oh, and a 4th side-issue:
Should each PHY have its own file? Or should we dump all the PHY drivers in one file? And if so, should THAT file be separate from the mii bus implementation file?

Andy Fleming
Open Source Team
Freescale Semiconductor, Inc

(Just an off-the-cuff answer here)

In line with the OCP->platform work I've been doing, I would think that creating 'phy' devices on the platform bus would be appropriate, with 'platform_data' that describes (a) the platform device ethernet it's bus is on and (b) it's PHY ID on that bus. The PHY's IRQ would be in it's platform resources.

Hmm. Don't really know from a sysfs perspective...

A PHY 'platform_data' struct like:

struct phy_device_data {
	struct {
		const char *name;
		int id;
	} ethernet_platform_device_parent;
	int	phy_id;
}

Actually, each PHY should have it's own device directory, like every other device. Eventually, PHYs should have /dev/phy* entries, where user-space can read/write PHY registers.

Jason McMullan <[email protected]>

So you would have each PHY know the controller to which it's attached? I would have thought the other way around... Hm. I will definitely have to read up on my driver model stuff

I think you misunderstood. Are you talking about sysfs? I was talking about actual source files. i.e. should there be dm9161.c, m88e1101.c, cis8201.c, etc.

Also, do we need user-space to read/write PHY registers. ethtool has this capability, I believe, and the interfaces there are settled.

Andy Fleming

Yes, I am talking about sysfs. And yes, I think every PHY should have
it's own .c file. (although most people could get away with
using a non-IRQ 'drivers/net/phy/phy-generic.c'

Doh! I forgot.

Jason McMullan <[email protected]>

Unfortunately, this is all quite platform specific and the ethernet driver may be the only one to know what to do here... add to that various special cases of the way the PHY is wired or controlled, I think we can't completely avoid special casing...

The ethernet driver can instanciate the PHYs at it's childs, though the case of several MACs sharing PHYs will be difficult to represent...

I would have them instanciated by the ethernet driver. Besides, the PHY driver will need to be able to identify it's "parent" driver in some ways to deal with special cases. It would be nice to have a library of utility code to independently deal with link tracking (basically what drivers like sungem do independently), with a callback to the ethernet driver to inform it of actual changes (notifier ?). MACs often have autopoll features and PHY often have interrupts, but from experience, that's not very useful and a good old timer based polling tend to do a better job most of the time.

I'd put all bcm5xxx in the same file ... they can be put together by families...

Also,

I think *requiring* a binding to ethernet devices is a bad idea.

For example, on many MPC8260 embedded systems, the MII bus is controlled by GPIO pins, not an ethernet MDIO system.

Some applications may use the MII bus to control non-PHY devices, such as Broadcom ethernet switches. I'm working for a general MII bus that ethernet PHYs and other devices can all use.

Jason McMullan <[email protected]>

Well, under the system I'm currently envisioning, the driver would be able to provide the data needed by the mii bus, but the hope would be to enable board files (for when the PHY is soldered on the motherboard, and the enet is not -- like on an MPC85xx) to provide this information instead, and leave out the enet as middleman.

I really don't want the driver to intantiate PHYs directly. The PHY is its own device, and the less net drivers have to understand their inner workings, the better. However, I hadn't considered the possibility of multiple MACs sharing the same PHY. It does, as you say, support my argument, though. With some careful design, the mii bus should be able to handle this type of setup easily.

One of my goals, personally, is to allow multiple net drivers to share the same mii bus, as in the case of the FCC enet controllers' PHYs on an 8560 ADS, which can be accessed through TSEC1's MII Management bus.

So when you say instantiated, would you consider calling an "attach" function with the phy_id and bus_id of the desired PHY instantiation? I'm fine with that. The PHY would need to be able to send notifications to the enet controller (currently done through a callback). I'm interested in ideas on how the notifier could be used (I have a distaste for callbacks).

Autopoll features sound pretty neat. I think the system should support that. PHY interrupts are supported (they work quite well on my 85xx system), as is timer-based polling. Do you really think that there are special cases which can't be handled using a library similar to the sungem_phy one?

Yeah, that sounds good.

Andy Fleming
Open Source Team
Freescale Semiconductor, Inc

Look at the notifier lists in include/linux/notifier.h

But that becomes MAC-dependant again... That means you'd need 1) a way for the MAC driver to ask the PHY driver what register it wants autopolled, and a function in the PHY driver for the MAC to call when it detects a change. Also, autopoll is broken in some MACs...

Nope. I think timer based polling with a sungem-like fallback mecanism to forced speeds would be nice.

Ben.

Ok, will do.

What I'm envisioning here is that the driver would be able to tell the PHY infrastructure that it's going to do its own thing, and then make use of the reading/configuring part of the infrastructure, similar to how sungem and gianfar are currently set up. But they would have the option of letting the infrastructure also handle the status updates. And, of course, the driver would not go through the effort to use autopoll if it were broken.

Yes, I agree. The system I currently have does fallback to forced, though it doesn't yet support the "magic aneg" feature you mentioned. But that should be easy to add, and so it shall be.

Andy Fleming

I've added pluggable PHY support modules and a bitbanger MII bus framework.

New patch at:
http://www.evillabs.net/~gus/patches/driver-mii-bus.patch

RPX/EP8260 MII bus example for bit-banger:

	http://www.evillabs.net/~gus/patches/rpx8260_mii.c

Jason McMullan <[email protected]>

Ok, given previous input, I now release mii-bus 2.0

* CIS8201 interrupt support
* The PHY device api is now similar to 'sungem'
* Set up your PHY as autoneg or forced speeds.

Jason McMullan <[email protected]>

#### Auto-generated patch ####

Date:          Thu, 02 Dec 2004 13:20:49 -0500
Description:   MII Bus interface

Depends:

###############################

Index of changes:

 drivers/net/Makefile            |    4 
 linux/drivers/net/mii_bitbang.c |  134 ++++++++
 linux/drivers/net/mii_bitbang.h |   40 ++
 linux/drivers/net/mii_bus.c     |  639 ++++++++++++++++++++++++++++++++++++++++
 linux/drivers/net/phy_cicada.c  |  177 +++++++++++
 linux/drivers/net/phy_davicom.c |  140 ++++++++
 linux/drivers/net/phy_lxt97x.c  |  210 +++++++++++++
 linux/drivers/net/phy_marvell.c |  125 +++++++
 linux/include/linux/mii_bus.h   |  191 +++++++++++
 9 files changed, 1659 insertions(+), 1 deletion(-)

@@ -62,7 +62,9 @@
 # end link order section
 #
 
-obj-$(CONFIG_MII) += mii.o
+obj-$(CONFIG_MII) += mii.o mii_bus.o mii_bitbang.o \
+		     phy_davicom.o phy_marvell.o phy_cicada.o \
+		     phy_lxt97x.o
 
 obj-$(CONFIG_SUNDANCE) += sundance.o
 obj-$(CONFIG_HAMACHI) += hamachi.o

@@ -0,0 +1,40 @@
+/* 
+ * drivers/net/mii_bitbang.h
+ *
+ * Author: Jason McMullan
+ *
+ * Copyright (c) 2004 Timesys Corp.
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ *
+ */
+#ifndef _NET_MII_BITBANG_H
+#define _NET_MII_BITBANG_H
+
+#include <linux/mii_bus.h>
+
+struct mii_bitbang {
+	const char *name;	/* Name of device */
+	void *priv;		/* Private data */
+
+	void (*send)(void *priv, int bit);	/* Send one bit */
+	int (*recv)(void *priv);		/* Recv one bit */
+	void (*reset)(void *priv);
+
+	/* Auto-filled-in information */
+	struct mii_bus bus;
+};
+
+/* Creates a bitbang MII bus
+ * Returns < 0 on error, otherwise a bus ID
+ */
+extern int mii_bitbang_register(struct mii_bitbang *info);
+
+/* Unregisters a bitbang MII bus
+ */
+extern void mii_bitbang_unregister(struct mii_bitbang *info);
+
+#endif /* _NET_MII_BITBANG_H */