On Thu, Jan 7, 2010 at 2:01 AM, Heiko Schocher hs@denx.de wrote:
If using UCC as Ethernet Controller and type = FAST_ETH, it was not possible to switch between 10 and 100 MBit interfaces. This patch adds this for following interfaces:
10_MII 10_RMII 10_RGMII 100_MII 100_RMII 100_RGMII
Signed-off-by: Heiko Schocher hs@denx.de
drivers/qe/uec.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 73 insertions(+), 0 deletions(-)
diff --git a/drivers/qe/uec.c b/drivers/qe/uec.c index db95ada..9851cc4 100644 --- a/drivers/qe/uec.c +++ b/drivers/qe/uec.c @@ -65,6 +65,22 @@ static uec_info_t uec_info[] = {
#define MAXCONTROLLERS (8)
+static char *enet_interface_name[] = {
- "10_MII",
- "10_RMII",
- "10_RGMII",
- "100_MII",
- "100_RMII",
- "100_RGMII",
- "1000_GMII",
- "1000_RGMII",
- "1000_RGMII_ID",
- "1000_RGMII_RXID",
- "1000_TBI",
- "1000_RTBI",
- "1000_SGMII"
+};
static struct eth_device *devlist[MAXCONTROLLERS];
u16 phy_read (struct uec_mii_info *mii_info, u16 regnum); @@ -497,6 +513,60 @@ bus_fail: return err; }
+static void adjust_fast_enet_interface(int speed, struct eth_device *dev) +{
- uec_private_t *uec = (uec_private_t *)dev->priv;
- uec_t *uec_regs;
- int change = 0;
- extern void change_phy_interface_mode(struct eth_device *dev,
- enet_interface_e mode);
- uec_regs = uec->uec_regs;
- switch (speed) {
- case 100:
- switch (uec->uec_info->enet_interface) {
- case ENET_10_MII:
- case ENET_10_RMII:
- case ENET_10_RGMII:
I know you aren't responsible for this design choice, but I'd love it if we stopped this sort of thing. There's no sensible reason to unite speed and interface type into one variable. They are, in fact, two nearly-independent variables. If they directly corresponded to some register value, it might make sense, but really they encourage a rigidity of thought about a concept which is much more fluid. The speed will vary based on the link partner. The interface is usually hardwired by the board designer, and at most, modifiable at boot time by DIP switches.
- uec->uec_info->enet_interface += 3;
And then we end up with something *highly* fragile like this. *3* has no meaning, except in the very specific context of the particular order in which the interfaces have been specified in this driver. All it takes is some architect deciding to add support for 10M SGMII, or 10M WHATEVERMII, and now everything breaks in a way that is not immediately apparent....
Andy