[U-Boot-Users] [PATCH] PPC4xx: Add Ethernet 1000BASE-X support for PPC4xx
This patch adds a new switch: "CONFIG_PHY_DYNAMIC_ANEG". When this symbol is defined, the PHY will advertise it's capabilities for autonegotiation based on the capabilities shown in the PHY's status registers, including 1000BASE-X. When "CONFIG_PHY_DYNAMIC_ANEG" is not defined, the PHY will advertise hard-coded capabilities, as before.
Signed-off-by: Larry Johnson lrj@acm.org --- diff --git a/common/miiphyutil.c b/common/miiphyutil.c index c69501f..39e3c20 100644 --- a/common/miiphyutil.c +++ b/common/miiphyutil.c @@ -36,7 +36,6 @@ #include <net.h>
/* local debug macro */ -#define MII_DEBUG #undef MII_DEBUG
#undef debug @@ -49,10 +48,10 @@ struct mii_dev { struct list_head link; char *name; - int (* read)(char *devname, unsigned char addr, - unsigned char reg, unsigned short *value); - int (* write)(char *devname, unsigned char addr, - unsigned char reg, unsigned short value); + int (*read) (char *devname, unsigned char addr, + unsigned char reg, unsigned short *value); + int (*write) (char *devname, unsigned char addr, + unsigned char reg, unsigned short value); };
static struct list_head mii_devs; @@ -62,21 +61,21 @@ static struct mii_dev *current_mii; * * Initialize global data. Need to be called before any other miiphy routine. */ -void miiphy_init() +void miiphy_init () { - INIT_LIST_HEAD(&mii_devs); - current_mii = NULL; + INIT_LIST_HEAD (&mii_devs); + current_mii = NULL; }
/***************************************************************************** * * Register read and write MII access routines for the device <name>. */ -void miiphy_register(char *name, - int (* read)(char *devname, unsigned char addr, - unsigned char reg, unsigned short *value), - int (* write)(char *devname, unsigned char addr, - unsigned char reg, unsigned short value)) +void miiphy_register (char *name, + int (*read) (char *devname, unsigned char addr, + unsigned char reg, unsigned short *value), + int (*write) (char *devname, unsigned char addr, + unsigned char reg, unsigned short value)) { struct list_head *entry; struct mii_dev *new_dev; @@ -84,63 +83,64 @@ void miiphy_register(char *name, unsigned int name_len;
/* check if we have unique name */ - list_for_each(entry, &mii_devs) { - miidev = list_entry(entry, struct mii_dev, link); - if (strcmp(miidev->name, name) == 0) { - printf("miiphy_register: non unique device name '%s'\n", - name); + list_for_each (entry, &mii_devs) { + miidev = list_entry (entry, struct mii_dev, link); + if (strcmp (miidev->name, name) == 0) { + printf ("miiphy_register: non unique device name " + "'%s'\n", name); return; } }
/* allocate memory */ - name_len = strlen(name); - new_dev = (struct mii_dev *)malloc(sizeof(struct mii_dev) + name_len + 1); + name_len = strlen (name); + new_dev = + (struct mii_dev *)malloc (sizeof (struct mii_dev) + name_len + 1);
- if(new_dev == NULL) { - printf("miiphy_register: cannot allocate memory for '%s'\n", - name); + if (new_dev == NULL) { + printf ("miiphy_register: cannot allocate memory for '%s'\n", + name); return; } - memset(new_dev, 0, sizeof(struct mii_dev) + name_len); + memset (new_dev, 0, sizeof (struct mii_dev) + name_len);
/* initalize mii_dev struct fields */ - INIT_LIST_HEAD(&new_dev->link); + INIT_LIST_HEAD (&new_dev->link); new_dev->read = read; new_dev->write = write; new_dev->name = (char *)(new_dev + 1); - strncpy(new_dev->name, name, name_len); + strncpy (new_dev->name, name, name_len); new_dev->name[name_len] = '\0';
- debug("miiphy_register: added '%s', read=0x%08lx, write=0x%08lx\n", - new_dev->name, new_dev->read, new_dev->write); + debug ("miiphy_register: added '%s', read=0x%08lx, write=0x%08lx\n", + new_dev->name, new_dev->read, new_dev->write);
/* add it to the list */ - list_add_tail(&new_dev->link, &mii_devs); + list_add_tail (&new_dev->link, &mii_devs);
if (!current_mii) current_mii = new_dev; }
-int miiphy_set_current_dev(char *devname) +int miiphy_set_current_dev (char *devname) { struct list_head *entry; struct mii_dev *dev;
- list_for_each(entry, &mii_devs) { - dev = list_entry(entry, struct mii_dev, link); + list_for_each (entry, &mii_devs) { + dev = list_entry (entry, struct mii_dev, link);
- if (strcmp(devname, dev->name) == 0) { + if (strcmp (devname, dev->name) == 0) { current_mii = dev; return 0; } }
- printf("No such device: %s\n", devname); + printf ("No such device: %s\n", devname); return 1; }
-char *miiphy_get_current_dev() +char *miiphy_get_current_dev () { if (current_mii) return current_mii->name; @@ -156,8 +156,8 @@ char *miiphy_get_current_dev() * Returns: * 0 on success */ -int miiphy_read(char *devname, unsigned char addr, unsigned char reg, - unsigned short *value) +int miiphy_read (char *devname, unsigned char addr, unsigned char reg, + unsigned short *value) { struct list_head *entry; struct mii_dev *dev; @@ -165,22 +165,22 @@ int miiphy_read(char *devname, unsigned char addr, unsigned char reg, int read_ret = 0;
if (!devname) { - printf("NULL device name!\n"); + printf ("NULL device name!\n"); return 1; }
- list_for_each(entry, &mii_devs) { - dev = list_entry(entry, struct mii_dev, link); + list_for_each (entry, &mii_devs) { + dev = list_entry (entry, struct mii_dev, link);
- if (strcmp(devname, dev->name) == 0) { + if (strcmp (devname, dev->name) == 0) { found_dev = 1; - read_ret = dev->read(devname, addr, reg, value); + read_ret = dev->read (devname, addr, reg, value); break; } }
if (found_dev == 0) - printf("No such device: %s\n", devname); + printf ("No such device: %s\n", devname);
return ((found_dev) ? read_ret : 1); } @@ -193,8 +193,8 @@ int miiphy_read(char *devname, unsigned char addr, unsigned char reg, * Returns: * 0 on success */ -int miiphy_write(char *devname, unsigned char addr, unsigned char reg, - unsigned short value) +int miiphy_write (char *devname, unsigned char addr, unsigned char reg, + unsigned short value) { struct list_head *entry; struct mii_dev *dev; @@ -202,22 +202,22 @@ int miiphy_write(char *devname, unsigned char addr, unsigned char reg, int write_ret = 0;
if (!devname) { - printf("NULL device name!\n"); + printf ("NULL device name!\n"); return 1; }
- list_for_each(entry, &mii_devs) { - dev = list_entry(entry, struct mii_dev, link); + list_for_each (entry, &mii_devs) { + dev = list_entry (entry, struct mii_dev, link);
- if (strcmp(devname, dev->name) == 0) { + if (strcmp (devname, dev->name) == 0) { found_dev = 1; - write_ret = dev->write(devname, addr, reg, value); + write_ret = dev->write (devname, addr, reg, value); break; } }
if (found_dev == 0) - printf("No such device: %s\n", devname); + printf ("No such device: %s\n", devname);
return ((found_dev) ? write_ret : 1); } @@ -226,23 +226,22 @@ int miiphy_write(char *devname, unsigned char addr, unsigned char reg, * * Print out list of registered MII capable devices. */ -void miiphy_listdev(void) +void miiphy_listdev (void) { struct list_head *entry; struct mii_dev *dev;
- puts("MII devices: "); - list_for_each(entry, &mii_devs) { - dev = list_entry(entry, struct mii_dev, link); - printf("'%s' ", dev->name); + puts ("MII devices: "); + list_for_each (entry, &mii_devs) { + dev = list_entry (entry, struct mii_dev, link); + printf ("'%s' ", dev->name); } - puts("\n"); + puts ("\n");
if (current_mii) - printf("Current device: '%s'\n", current_mii->name); + printf ("Current device: '%s'\n", current_mii->name); }
- /***************************************************************************** * * Read the OUI, manufacture's model number, and revision number. @@ -254,9 +253,7 @@ void miiphy_listdev(void) * Returns: * 0 on success */ -int miiphy_info (char *devname, - unsigned char addr, - unsigned int *oui, +int miiphy_info (char *devname, unsigned char addr, unsigned int *oui, unsigned char *model, unsigned char *rev) { unsigned int reg = 0; @@ -288,13 +285,12 @@ int miiphy_info (char *devname, #ifdef DEBUG printf ("PHY_PHYIDR[1,2] @ 0x%x = 0x%08x\n", addr, reg); #endif - *oui = ( reg >> 10); - *model = (unsigned char) ((reg >> 4) & 0x0000003F); - *rev = (unsigned char) ( reg & 0x0000000F); + *oui = (reg >> 10); + *model = (unsigned char)((reg >> 4) & 0x0000003F); + *rev = (unsigned char)(reg & 0x0000000F); return (0); }
- /***************************************************************************** * * Reset the PHY. @@ -345,104 +341,150 @@ int miiphy_reset (char *devname, unsigned char addr) return (0); }
- /***************************************************************************** * - * Determine the ethernet speed (10/100). + * Determine the ethernet speed (10/100/1000). Return 10 on error. */ int miiphy_speed (char *devname, unsigned char addr) { - unsigned short reg; + u16 bmcr;
#if defined(CONFIG_PHY_GIGE) - if (miiphy_read (devname, addr, PHY_1000BTSR, ®)) { - printf ("PHY 1000BT Status read failed\n"); - } else { - if (reg != 0xFFFF) { - if ((reg & (PHY_1000BTSR_1000FD | PHY_1000BTSR_1000HD)) !=0) { - return (_1000BASET); - } + u16 btsr; + +#if defined(CONFIG_PHY_DYNAMIC_ANEG) + u16 bmsr; + + /* Check for 1000BASE-X. */ + if (miiphy_read (devname, addr, PHY_BMSR, &bmsr)) { + printf ("PHY status"); + goto miiphy_read_failed; + } + if (bmsr & PHY_BMSR_EXT_STAT) { + u16 exsr; + + if (miiphy_read (devname, addr, PHY_EXSR, &exsr)) { + printf ("PHY extended status"); + goto miiphy_read_failed; + } + if (exsr & (PHY_EXSR_1000XF | PHY_EXSR_1000XH)) { + /* 1000BASE-X */ + return _1000BASET; } } +#endif /* defined(CONFIG_PHY_DYNAMIC_ANEG) */ + + /* Check for 1000BASE-T. */ + if (miiphy_read (devname, addr, PHY_1000BTSR, &btsr)) { + printf ("PHY 1000BT status"); + goto miiphy_read_failed; + } + if (btsr != 0xFFFF && + (btsr & (PHY_1000BTSR_1000FD | PHY_1000BTSR_1000HD))) { + return _1000BASET; + } #endif /* CONFIG_PHY_GIGE */
/* Check Basic Management Control Register first. */ - if (miiphy_read (devname, addr, PHY_BMCR, ®)) { - puts ("PHY speed read failed, assuming 10bT\n"); - return (_10BASET); + if (miiphy_read (devname, addr, PHY_BMCR, &bmcr)) { + printf ("PHY speed"); + goto miiphy_read_failed; } /* Check if auto-negotiation is on. */ - if ((reg & PHY_BMCR_AUTON) != 0) { + if (bmcr & PHY_BMCR_AUTON) { /* Get auto-negotiation results. */ - if (miiphy_read (devname, addr, PHY_ANLPAR, ®)) { - puts ("PHY AN speed read failed, assuming 10bT\n"); - return (_10BASET); - } - if ((reg & PHY_ANLPAR_100) != 0) { - return (_100BASET); - } else { - return (_10BASET); + u16 anlpar; + + if (miiphy_read (devname, addr, PHY_ANLPAR, &anlpar)) { + printf ("PHY AN speed"); + goto miiphy_read_failed; } + return (anlpar & PHY_ANLPAR_100) ? _100BASET : _10BASET; } /* Get speed from basic control settings. */ - else if (reg & PHY_BMCR_100MB) { - return (_100BASET); - } else { - return (_10BASET); - } + return (bmcr & PHY_BMCR_100MB) ? _100BASET : _10BASET;
+ miiphy_read_failed: + printf (" read failed, assuming 10BASE-T\n"); + return _10BASET; }
- /***************************************************************************** * - * Determine full/half duplex. + * Determine full/half duplex. Return half on error. */ int miiphy_duplex (char *devname, unsigned char addr) { - unsigned short reg; + u16 bmcr;
#if defined(CONFIG_PHY_GIGE) - if (miiphy_read (devname, addr, PHY_1000BTSR, ®)) { - printf ("PHY 1000BT Status read failed\n"); - } else { - if ( (reg != 0xFFFF) && - (reg & (PHY_1000BTSR_1000FD | PHY_1000BTSR_1000HD)) ) { - if ((reg & PHY_1000BTSR_1000FD) !=0) { - return (FULL); - } else { - return (HALF); + u16 btsr; + +#if defined(CONFIG_PHY_DYNAMIC_ANEG) + u16 bmsr; + + /* Check for 1000BASE-X. */ + if (miiphy_read (devname, addr, PHY_BMSR, &bmsr)) { + printf ("PHY status"); + goto miiphy_read_failed; + } + if (bmsr & PHY_BMSR_EXT_STAT) { + u16 exsr; + + if (miiphy_read (devname, addr, PHY_EXSR, &exsr)) { + printf ("PHY extended status"); + goto miiphy_read_failed; + } + if (exsr & (PHY_EXSR_1000XF | PHY_EXSR_1000XH)) { + /* 1000BASE-X */ + u16 anlpar; + + if (miiphy_read (devname, addr, PHY_ANLPAR, &anlpar)) { + printf ("1000BASE-X PHY AN duplex"); + goto miiphy_read_failed; } + return (anlpar & PHY_X_ANLPAR_FD) ? FULL : HALF; + } + } +#endif /* defined(CONFIG_PHY_DYNAMIC_ANEG) */ + + /* Check for 1000BASE-T. */ + if (miiphy_read (devname, addr, PHY_1000BTSR, &btsr)) { + printf ("PHY 1000BT status"); + goto miiphy_read_failed; + } + if (btsr != 0xFFFF) { + if (btsr & PHY_1000BTSR_1000FD) { + return FULL; + } else if (btsr & PHY_1000BTSR_1000HD) { + return HALF; } } #endif /* CONFIG_PHY_GIGE */
/* Check Basic Management Control Register first. */ - if (miiphy_read (devname, addr, PHY_BMCR, ®)) { - puts ("PHY duplex read failed, assuming half duplex\n"); - return (HALF); + if (miiphy_read (devname, addr, PHY_BMCR, &bmcr)) { + puts ("PHY duplex"); + goto miiphy_read_failed; } /* Check if auto-negotiation is on. */ - if ((reg & PHY_BMCR_AUTON) != 0) { + if ((bmcr & PHY_BMCR_AUTON) != 0) { /* Get auto-negotiation results. */ - if (miiphy_read (devname, addr, PHY_ANLPAR, ®)) { - puts ("PHY AN duplex read failed, assuming half duplex\n"); - return (HALF); - } + u16 anlpar;
- if ((reg & (PHY_ANLPAR_10FD | PHY_ANLPAR_TXFD)) != 0) { - return (FULL); - } else { - return (HALF); + if (miiphy_read (devname, addr, PHY_ANLPAR, &anlpar)) { + puts ("PHY AN duplex"); + goto miiphy_read_failed; } + return (anlpar & (PHY_ANLPAR_10FD | PHY_ANLPAR_TXFD)) ? + FULL : HALF; } /* Get speed from basic control settings. */ - else if (reg & PHY_BMCR_DPLX) { - return (FULL); - } else { - return (HALF); - } + return (bmcr & PHY_BMCR_DPLX) ? FULL : HALF;
+ miiphy_read_failed: + printf (" read failed, assuming half duplex\n"); + return HALF; }
#ifdef CFG_FAULT_ECHO_LINK_DOWN @@ -455,7 +497,7 @@ int miiphy_link (char *devname, unsigned char addr) unsigned short reg;
/* dummy read; needed to latch some phys */ - (void)miiphy_read(devname, addr, PHY_BMSR, ®); + (void)miiphy_read (devname, addr, PHY_BMSR, ®); if (miiphy_read (devname, addr, PHY_BMSR, ®)) { puts ("PHY_BMSR read failed, assuming no link\n"); return (0); @@ -469,5 +511,4 @@ int miiphy_link (char *devname, unsigned char addr) } } #endif - #endif /* CONFIG_MII */ diff --git a/cpu/ppc4xx/miiphy.c b/cpu/ppc4xx/miiphy.c index 6b98025..55d777f 100644 --- a/cpu/ppc4xx/miiphy.c +++ b/cpu/ppc4xx/miiphy.c @@ -39,6 +39,9 @@ | 19-Jul-00 Ported to esd cpci405 sr | 23-Dec-03 Ported from miiphy.c to 440GX Travis Sawyer TBS | travis.sawyer@sandburst.com + | 20-Oct-07 Added support for autonegotiation base on reported lrj + | capabilities, including 1000BASE-X. Larry Johnson + | lrj@acm.org | +-----------------------------------------------------------------------------*/
@@ -60,7 +63,6 @@ void miiphy_dump (char *devname, unsigned char addr) unsigned long i; unsigned short data;
- for (i = 0; i < 0x1A; i++) { if (miiphy_read (devname, addr, i, &data)) { printf ("read error for reg %lx\n", i); @@ -75,15 +77,87 @@ void miiphy_dump (char *devname, unsigned char addr) } /* end for loop */ } /* end dump */
- /***********************************************************/ /* (Re)start autonegotiation */ /***********************************************************/ int phy_setup_aneg (char *devname, unsigned char addr) { - unsigned short ctl, adv; + u16 bmcr; + +#if defined(CONFIG_PHY_DYNAMIC_ANEG) + /* + * Set up advertisement based on capablilities reported by the PHY. + * This should work for both copper and fiber. + */ + u16 bmsr; +#if defined(CONFIG_PHY_GIGE) + u16 exsr = 0x0000; +#endif + + miiphy_read (devname, addr, PHY_BMSR, &bmsr); + +#if defined(CONFIG_PHY_GIGE) + if (bmsr & PHY_BMSR_EXT_STAT) { + miiphy_read (devname, addr, PHY_EXSR, &exsr); + } + + if (exsr & (PHY_EXSR_1000XF | PHY_EXSR_1000XH)) { + /* 1000BASE-X */ + u16 anar = 0x0000; + + if (exsr & PHY_EXSR_1000XF) { + anar |= PHY_X_ANLPAR_FD; + } + if (exsr & PHY_EXSR_1000XH) { + anar |= PHY_X_ANLPAR_HD; + } + miiphy_write (devname, addr, PHY_ANAR, anar); + } else +#endif + { + u16 anar, btcr; + + miiphy_read (devname, addr, PHY_ANAR, &anar); + anar &= ~(0x5000 | PHY_ANLPAR_T4 | PHY_ANLPAR_TXFD | + PHY_ANLPAR_TX | PHY_ANLPAR_10FD | PHY_ANLPAR_10); + + miiphy_read (devname, addr, PHY_1000BTCR, &btcr); + btcr &= ~(0x00FF | PHY_1000BTCR_1000FD | PHY_1000BTCR_1000HD); + + if (bmsr & PHY_BMSR_100T4) { + anar |= PHY_ANLPAR_T4; + } + if (bmsr & PHY_BMSR_100TXF) { + anar |= PHY_ANLPAR_TXFD; + } + if (bmsr & PHY_BMSR_100TXH) { + anar |= PHY_ANLPAR_TX; + } + if (bmsr & PHY_BMSR_10TF) { + anar |= PHY_ANLPAR_10FD; + } + if (bmsr & PHY_BMSR_10TH) { + anar |= PHY_ANLPAR_10; + } + miiphy_write (devname, addr, PHY_ANAR, anar); + +#if defined(CONFIG_PHY_GIGE) + if (exsr & PHY_EXSR_1000TF) { + btcr |= PHY_1000BTCR_1000FD; + } + if (exsr & PHY_EXSR_1000TH) { + btcr |= PHY_1000BTCR_1000HD; + } + miiphy_write (devname, addr, PHY_1000BTCR, btcr); +#endif + } + +#else /* defined(CONFIG_PHY_DYNAMIC_ANEG) */ + /* + * Set up standard advertisement + */ + u16 adv;
- /* Setup standard advertise */ miiphy_read (devname, addr, PHY_ANAR, &adv); adv |= (PHY_ANLPAR_ACK | PHY_ANLPAR_RF | PHY_ANLPAR_T4 | PHY_ANLPAR_TXFD | PHY_ANLPAR_TX | PHY_ANLPAR_10FD | @@ -94,15 +168,16 @@ int phy_setup_aneg (char *devname, unsigned char addr) adv |= (0x0300); miiphy_write (devname, addr, PHY_1000BTCR, adv);
+#endif /* defined(CONFIG_PHY_DYNAMIC_ANEG) */ + /* Start/Restart aneg */ - miiphy_read (devname, addr, PHY_BMCR, &ctl); - ctl |= (PHY_BMCR_AUTON | PHY_BMCR_RST_NEG); - miiphy_write (devname, addr, PHY_BMCR, ctl); + miiphy_read (devname, addr, PHY_BMCR, &bmcr); + bmcr |= (PHY_BMCR_AUTON | PHY_BMCR_RST_NEG); + miiphy_write (devname, addr, PHY_BMCR, bmcr);
return 0; }
- /***********************************************************/ /* read a phy reg and return the value with a rc */ /***********************************************************/ @@ -145,21 +220,20 @@ unsigned int miiphy_getemac_offset (void) #endif }
- -int emac4xx_miiphy_read (char *devname, unsigned char addr, - unsigned char reg, unsigned short *value) +int emac4xx_miiphy_read (char *devname, unsigned char addr, unsigned char reg, + unsigned short *value) { unsigned long sta_reg; /* STA scratch area */ unsigned long i; unsigned long emac_reg;
- emac_reg = miiphy_getemac_offset (); /* see if it is ready for 1000 nsec */ i = 0;
/* see if it is ready for sec */ - while ((in32 (EMAC_STACR + emac_reg) & EMAC_STACR_OC) == EMAC_STACR_OC_MASK) { + while ((in32 (EMAC_STACR + emac_reg) & EMAC_STACR_OC) == + EMAC_STACR_OC_MASK) { udelay (7); if (i > 5) { #ifdef ET_DEBUG @@ -175,10 +249,10 @@ int emac4xx_miiphy_read (char *devname, unsigned char addr, /* set clock (50Mhz) and read flags */ #if defined(CONFIG_440GX) || defined(CONFIG_440SPE) || \ defined(CONFIG_440EPX) || defined(CONFIG_440GRX) -#if defined(CONFIG_IBM_EMAC4_V4) /* EMAC4 V4 changed bit setting */ - sta_reg = (sta_reg & ~EMAC_STACR_OP_MASK) | EMAC_STACR_READ; +#if defined(CONFIG_IBM_EMAC4_V4) /* EMAC4 V4 changed bit setting */ + sta_reg = (sta_reg & ~EMAC_STACR_OP_MASK) | EMAC_STACR_READ; #else - sta_reg |= EMAC_STACR_READ; + sta_reg |= EMAC_STACR_READ; #endif #else sta_reg = (sta_reg | EMAC_STACR_READ) & ~EMAC_STACR_CLK_100MHZ; @@ -198,7 +272,7 @@ int emac4xx_miiphy_read (char *devname, unsigned char addr,
sta_reg = in32 (EMAC_STACR + emac_reg); #ifdef ET_DEBUG - printf ("a21: read : EMAC_STACR=0x%0x\n", sta_reg); /* test-only */ + printf ("a21: read : EMAC_STACR=0x%0x\n", sta_reg); /* test-only */ #endif i = 0; while ((sta_reg & EMAC_STACR_OC) == EMAC_STACR_OC_MASK) { @@ -216,19 +290,17 @@ int emac4xx_miiphy_read (char *devname, unsigned char addr, return -1; }
- *value = *(short *) (&sta_reg); + *value = *(short *)(&sta_reg); return 0;
- } /* phy_read */
- /***********************************************************/ /* write a phy reg and return the value with a rc */ /***********************************************************/
-int emac4xx_miiphy_write (char *devname, unsigned char addr, - unsigned char reg, unsigned short value) +int emac4xx_miiphy_write (char *devname, unsigned char addr, unsigned char reg, + unsigned short value) { unsigned long sta_reg; /* STA scratch area */ unsigned long i; @@ -238,7 +310,8 @@ int emac4xx_miiphy_write (char *devname, unsigned char addr, /* see if it is ready for 1000 nsec */ i = 0;
- while ((in32 (EMAC_STACR + emac_reg) & EMAC_STACR_OC) == EMAC_STACR_OC_MASK) { + while ((in32 (EMAC_STACR + emac_reg) & EMAC_STACR_OC) == + EMAC_STACR_OC_MASK) { if (i > 5) return -1; udelay (7); @@ -249,10 +322,10 @@ int emac4xx_miiphy_write (char *devname, unsigned char addr, /* set clock (50Mhz) and read flags */ #if defined(CONFIG_440GX) || defined(CONFIG_440SPE) || \ defined(CONFIG_440EPX) || defined(CONFIG_440GRX) -#if defined(CONFIG_IBM_EMAC4_V4) /* EMAC4 V4 changed bit setting */ - sta_reg = (sta_reg & ~EMAC_STACR_OP_MASK) | EMAC_STACR_WRITE; +#if defined(CONFIG_IBM_EMAC4_V4) /* EMAC4 V4 changed bit setting */ + sta_reg = (sta_reg & ~EMAC_STACR_OP_MASK) | EMAC_STACR_WRITE; #else - sta_reg |= EMAC_STACR_WRITE; + sta_reg |= EMAC_STACR_WRITE; #endif #else sta_reg = (sta_reg | EMAC_STACR_WRITE) & ~EMAC_STACR_CLK_100MHZ; @@ -263,8 +336,8 @@ int emac4xx_miiphy_write (char *devname, unsigned char addr, !defined(CONFIG_440EPX) && !defined(CONFIG_440GRX) sta_reg = sta_reg | CONFIG_PHY_CLK_FREQ; /* Set clock frequency (PLB freq. dependend) */ #endif - sta_reg = sta_reg | ((unsigned long) addr << 5);/* Phy address */ - sta_reg = sta_reg | EMAC_STACR_OC_MASK; /* new IBM emac v4 */ + sta_reg = sta_reg | ((unsigned long)addr << 5); /* Phy address */ + sta_reg = sta_reg | EMAC_STACR_OC_MASK; /* new IBM emac v4 */ memcpy (&sta_reg, &value, 2); /* put in data */
out32 (EMAC_STACR + emac_reg, sta_reg); @@ -273,7 +346,7 @@ int emac4xx_miiphy_write (char *devname, unsigned char addr, i = 0; sta_reg = in32 (EMAC_STACR + emac_reg); #ifdef ET_DEBUG - printf ("a31: read : EMAC_STACR=0x%0x\n", sta_reg); /* test-only */ + printf ("a31: read : EMAC_STACR=0x%0x\n", sta_reg); /* test-only */ #endif while ((sta_reg & EMAC_STACR_OC) == EMAC_STACR_OC_MASK) { udelay (7); diff --git a/include/miiphy.h b/include/miiphy.h index 71716b0..0332e4b 100644 --- a/include/miiphy.h +++ b/include/miiphy.h @@ -33,6 +33,7 @@ | 04-May-99 Created MKW | 07-Jul-99 Added full duplex support MKW | 08-Sep-01 Tweaks gvb +| 28-Oct-07 Added 1000BASE-X support. Larry Johnson lrj@acm.org lrj | +----------------------------------------------------------------------------*/ #ifndef _miiphy_h_ @@ -40,26 +41,26 @@
#include <net.h>
-int miiphy_read(char *devname, unsigned char addr, unsigned char reg, +int miiphy_read(char *devname, unsigned char addr, unsigned char reg, unsigned short *value); -int miiphy_write(char *devname, unsigned char addr, unsigned char reg, - unsigned short value); -int miiphy_info(char *devname, unsigned char addr, unsigned int *oui, +int miiphy_write(char *devname, unsigned char addr, unsigned char reg, + unsigned short value); +int miiphy_info(char *devname, unsigned char addr, unsigned int *oui, unsigned char *model, unsigned char *rev); -int miiphy_reset(char *devname, unsigned char addr); -int miiphy_speed(char *devname, unsigned char addr); -int miiphy_duplex(char *devname, unsigned char addr); +int miiphy_reset(char *devname, unsigned char addr); +int miiphy_speed(char *devname, unsigned char addr); +int miiphy_duplex(char *devname, unsigned char addr); #ifdef CFG_FAULT_ECHO_LINK_DOWN -int miiphy_link(char *devname, unsigned char addr); +int miiphy_link(char *devname, unsigned char addr); #endif
void miiphy_init(void);
void miiphy_register(char *devname, - int (* read)(char *devname, unsigned char addr, - unsigned char reg, unsigned short *value), - int (* write)(char *devname, unsigned char addr, - unsigned char reg, unsigned short value)); + int (*read) (char *devname, unsigned char addr, + unsigned char reg, unsigned short *value), + int (*write) (char *devname, unsigned char addr, + unsigned char reg, unsigned short value));
int miiphy_set_current_dev(char *devname); char *miiphy_get_current_dev(void); @@ -68,14 +69,14 @@ void miiphy_listdev(void);
#define BB_MII_DEVNAME "bbmii"
-int bb_miiphy_read (char *devname, unsigned char addr, - unsigned char reg, unsigned short *value); -int bb_miiphy_write (char *devname, unsigned char addr, - unsigned char reg, unsigned short value); +int bb_miiphy_read(char *devname, unsigned char addr, + unsigned char reg, unsigned short *value); +int bb_miiphy_write(char *devname, unsigned char addr, + unsigned char reg, unsigned short value);
/* phy seed setup */ #define AUTO 99 -#define _1000BASET 1000 +#define _1000BASET 1000 #define _100BASET 100 #define _10BASET 10 #define HALF 22 @@ -90,9 +91,10 @@ int bb_miiphy_write (char *devname, unsigned char addr, #define PHY_ANLPAR 0x05 #define PHY_ANER 0x06 #define PHY_ANNPTR 0x07 -#define PHY_ANLPNP 0x08 -#define PHY_1000BTCR 0x09 -#define PHY_1000BTSR 0x0A +#define PHY_ANLPNP 0x08 +#define PHY_1000BTCR 0x09 +#define PHY_1000BTSR 0x0A +#define PHY_EXSR 0x0F #define PHY_PHYSTS 0x10 #define PHY_MIPSCR 0x11 #define PHY_MIPGSR 0x12 @@ -126,6 +128,7 @@ int bb_miiphy_write (char *devname, unsigned char addr, #define PHY_BMSR_100TXH 0x2000 #define PHY_BMSR_10TF 0x1000 #define PHY_BMSR_10TH 0x0800 +#define PHY_BMSR_EXT_STAT 0x0100 #define PHY_BMSR_PRE_SUP 0x0040 #define PHY_BMSR_AUTN_COMP 0x0020 #define PHY_BMSR_RF 0x0010 @@ -138,18 +141,31 @@ int bb_miiphy_write (char *devname, unsigned char addr, #define PHY_ANLPAR_NP 0x8000 #define PHY_ANLPAR_ACK 0x4000 #define PHY_ANLPAR_RF 0x2000 +#define PHY_ANLPAR_ASYMP 0x0800 +#define PHY_ANLPAR_PAUSE 0x0400 #define PHY_ANLPAR_T4 0x0200 #define PHY_ANLPAR_TXFD 0x0100 #define PHY_ANLPAR_TX 0x0080 #define PHY_ANLPAR_10FD 0x0040 #define PHY_ANLPAR_10 0x0020 -#define PHY_ANLPAR_100 0x0380 /* we can run at 100 */ +#define PHY_ANLPAR_100 0x0380 /* we can run at 100 */ +/* phy ANLPAR 1000BASE-X */ +#define PHY_X_ANLPAR_NP 0x8000 +#define PHY_X_ANLPAR_ACK 0x4000 +#define PHY_X_ANLPAR_RF_MASK 0x3000 +#define PHY_X_ANLPAR_PAUSE_MASK 0x0180 +#define PHY_X_ANLPAR_HD 0x0040 +#define PHY_X_ANLPAR_FD 0x0020
#define PHY_ANLPAR_PSB_MASK 0x001f #define PHY_ANLPAR_PSB_802_3 0x0001 #define PHY_ANLPAR_PSB_802_9 0x0002
-/* PHY_1000BTSR */ +/* phy 1000BTCR */ +#define PHY_1000BTCR_1000FD 0x0200 +#define PHY_1000BTCR_1000HD 0x0100 + +/* phy 1000BTSR */ #define PHY_1000BTSR_MSCF 0x8000 #define PHY_1000BTSR_MSCR 0x4000 #define PHY_1000BTSR_LRS 0x2000 @@ -157,4 +173,10 @@ int bb_miiphy_write (char *devname, unsigned char addr, #define PHY_1000BTSR_1000FD 0x0800 #define PHY_1000BTSR_1000HD 0x0400
+/* phy EXSR */ +#define PHY_EXSR_1000XF 0x8000 +#define PHY_EXSR_1000XH 0x4000 +#define PHY_EXSR_1000TF 0x2000 +#define PHY_EXSR_1000TH 0x1000 + #endif
Larry Johnson wrote:
This patch adds a new switch: "CONFIG_PHY_DYNAMIC_ANEG". When this symbol is defined, the PHY will advertise it's capabilities for autonegotiation based on the capabilities shown in the PHY's status registers, including 1000BASE-X. When "CONFIG_PHY_DYNAMIC_ANEG" is not defined, the PHY will advertise hard-coded capabilities, as before.
I won't address the content yet, just the cosmetic changes that make up the bulk of this patch. I think you've done a good thing by running Lindent (or whatever) on all the files you've touched, but it has the effect of obscuring the meat of your work. No need to re-do it this time, but IMHO, purely cosmetic changes should be separate patches and labeled as such.
Of course, that's just MHO, and others may feel differently. Thoughts?
regards, Ben
Ben Warren wrote:
Larry Johnson wrote:
This patch adds a new switch: "CONFIG_PHY_DYNAMIC_ANEG". When this symbol is defined, the PHY will advertise it's capabilities for autonegotiation based on the capabilities shown in the PHY's status registers, including 1000BASE-X. When "CONFIG_PHY_DYNAMIC_ANEG" is not defined, the PHY will advertise hard-coded capabilities, as before.
I won't address the content yet, just the cosmetic changes that make up the bulk of this patch. I think you've done a good thing by running Lindent (or whatever) on all the files you've touched, but it has the effect of obscuring the meat of your work. No need to re-do it this time, but IMHO, purely cosmetic changes should be separate patches and labeled as such.
Of course, that's just MHO, and others may feel differently. Thoughts?
regards, Ben
Thanks for bringing this up. I noticed the same probelm, but couldn't think of how to handle it. I want to run Lindent on my changes to fix any issues with them, especially as the U-Boot/Linux style is so different from what I'm used to. I thought about running Lindent and copying just my changes back to the original, but that seems error prone (and too much work).
Would it be better to run Lindent on the original files, post those as changes, and then post a second patch with the new material?
BTW, I've noticed that the "Lindent -pcs" format is used inconsistently within files. Since I'm also making changes in Linux, it's hard to remember whether to type "foo(bar);" or "foo (bar);", maybe others have this difficulty, too. I've tried to use the form that is prevalent in each individual file, though some are pretty much split down the middle.
How do others feel?
Regards, Larry
On Mon, 2007-10-29 at 13:41, Larry Johnson wrote:
Would it be better to run Lindent on the original files, post those as changes, and then post a second patch with the new material?
In my opinion, yes, please.
BTW, I've noticed that the "Lindent -pcs" format is used inconsistently within files. Since I'm also making changes in Linux, it's hard to remember whether to type "foo(bar);" or "foo (bar);",
In my opinion, "foo (bar)" is bad style, and I prefer the Linux form, "foo(bar)".
jdl
On Monday 29 October 2007, Jon Loeliger wrote:
On Mon, 2007-10-29 at 13:41, Larry Johnson wrote:
Would it be better to run Lindent on the original files, post those as changes, and then post a second patch with the new material?
In my opinion, yes, please.
Yes, please do the coding style cleanup first, and the "real" patch later in a separate commit. Otherwise it will very hard to make out your changes in the commit-diff.
BTW, I've noticed that the "Lindent -pcs" format is used inconsistently within files. Since I'm also making changes in Linux, it's hard to remember whether to type "foo(bar);" or "foo (bar);",
In my opinion, "foo (bar)" is bad style, and I prefer the Linux form, "foo(bar)".
This is my way of coding too. Mainly because I'm used to doing it this way and Linux uses it too. But I know that Wolfgang prefers the "other" style. My vote would be, when you really run Lindent on this file (or other), switch to the Linux style completely.
Best regards, Stefan
===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de =====================================================================
Stefan Roese wrote:
On Monday 29 October 2007, Jon Loeliger wrote:
On Mon, 2007-10-29 at 13:41, Larry Johnson wrote:
Would it be better to run Lindent on the original files, post those as changes, and then post a second patch with the new material?
In my opinion, yes, please.
Yes, please do the coding style cleanup first, and the "real" patch later in a separate commit. Otherwise it will very hard to make out your changes in the commit-diff.
BTW, I've noticed that the "Lindent -pcs" format is used inconsistently within files. Since I'm also making changes in Linux, it's hard to remember whether to type "foo(bar);" or "foo (bar);",
In my opinion, "foo (bar)" is bad style, and I prefer the Linux form, "foo(bar)".
This is my way of coding too. Mainly because I'm used to doing it this way and Linux uses it too. But I know that Wolfgang prefers the "other" style. My vote would be, when you really run Lindent on this file (or other), switch to the Linux style completely.
Best regards, Stefan
===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de =====================================================================
Thanks everyone for the suggests. I will split the cosmetic from the functional patches and resubmit for further comments.
Best regards, Larry
Jon Loeliger wrote:
On Mon, 2007-10-29 at 13:41, Larry Johnson wrote:
Would it be better to run Lindent on the original files, post those as changes, and then post a second patch with the new material?
In my opinion, yes, please.
BTW, I've noticed that the "Lindent -pcs" format is used inconsistently within files. Since I'm also making changes in Linux, it's hard to remember whether to type "foo(bar);" or "foo (bar);",
In my opinion, "foo (bar)" is bad style, and I prefer the Linux form, "foo(bar)".
jdl
Yeah, in Larry's patch, Lindent mostly created the 'bad-style' format. I thought the idea behind Lindent was that you didn't need to figure out all the appropriate switches for 'indent'. I don't know what '-pcs' does... Of course, my Lindent is old (c 2.6.19) and maybe it's been changed since then.
regards, Ben
In message 1193685290.1462.19.camel@ld0161-tx32 you wrote:
In my opinion, "foo (bar)" is bad style, and I prefer the Linux form, "foo(bar)".
That's your opinion, which I usually value, but here it will be ignored ;-)
Best regards,
Wolfgang Denk
In message 47262967.8070307@arlinx.com you wrote:
Would it be better to run Lindent on the original files, post those as changes, and then post a second patch with the new material?
If it's your code, or other original code, then yes, this is the way to go.
But if it's code that was "borrowed" from somewhere else (like from the Linux kernel, busybox, etc.) and then only slightly modified to make it work under U-Boot, then please *DO* *NOT* reformat it, as this basicly makes it impossible to update the file from new versions of the original code.
BTW, I've noticed that the "Lindent -pcs" format is used inconsistently within files. Since I'm also making changes in Linux, it's hard to remember whether to type "foo(bar);" or "foo (bar);", maybe others have this difficulty, too. I've tried to use the form that is prevalent in each individual file, though some are pretty much split down the middle.
How do others feel?
It should be "foo (bar)".
And all this should be sufficiently documented at http://www.denx.de/wiki/UBoot/CodingStyle
If you feel that details are missing or not clear enought in this text please help to extend / fix it.
Best regards,
Wolfgang Denk
Wolfgang Denk wrote:
In message 47262967.8070307@arlinx.com you wrote:
Would it be better to run Lindent on the original files, post those as changes, and then post a second patch with the new material?
If it's your code, or other original code, then yes, this is the way to go.
But if it's code that was "borrowed" from somewhere else (like from the Linux kernel, busybox, etc.) and then only slightly modified to make it work under U-Boot, then please *DO* *NOT* reformat it, as this basicly makes it impossible to update the file from new versions of the original code.
Because, in general, it's hard to know the source of the code, it sounds like the safest thing is for my to use Lindent only to identify problems the patch, and fix them manually.
BTW, I've noticed that the "Lindent -pcs" format is used inconsistently within files. Since I'm also making changes in Linux, it's hard to remember whether to type "foo(bar);" or "foo (bar);", maybe others have this difficulty, too. I've tried to use the form that is prevalent in each individual file, though some are pretty much split down the middle.
How do others feel?
It should be "foo (bar)".
And all this should be sufficiently documented at http://www.denx.de/wiki/UBoot/CodingStyle
If you feel that details are missing or not clear enought in this text please help to extend / fix it.
The text is clear, I just couldn't decide if this was "a custom more honour'd in the breach than the observance" :-).
Best regards,
Wolfgang Denk
Thanks for your help, I'll re-redo the patch.
Best regards, Larry
On Monday 29 October 2007, Larry Johnson wrote:
This patch adds a new switch: "CONFIG_PHY_DYNAMIC_ANEG". When this symbol is defined, the PHY will advertise it's capabilities for autonegotiation based on the capabilities shown in the PHY's status registers, including 1000BASE-X. When "CONFIG_PHY_DYNAMIC_ANEG" is not defined, the PHY will advertise hard-coded capabilities, as before.
Signed-off-by: Larry Johnson lrj@acm.org
<snip>
diff --git a/cpu/ppc4xx/miiphy.c b/cpu/ppc4xx/miiphy.c index 6b98025..55d777f 100644 --- a/cpu/ppc4xx/miiphy.c +++ b/cpu/ppc4xx/miiphy.c @@ -39,6 +39,9 @@
| 19-Jul-00 Ported to esd cpci405 sr | 23-Dec-03 Ported from miiphy.c to 440GX Travis Sawyer TBS | travis.sawyer@sandburst.com
- | 20-Oct-07 Added support for autonegotiation base on reported
lrj + | capabilities, including 1000BASE-X. Larry Johnson
Please don't add commit logs to the source files. I know it has been done before, but we use git to maintain such change messages. If you like you can remove the older change messages too in your next patch version.
Best regards, Stefan
===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de =====================================================================
Larry Johnson wrote:
This patch adds a new switch: "CONFIG_PHY_DYNAMIC_ANEG". When this symbol is defined, the PHY will advertise it's capabilities for autonegotiation based on the capabilities shown in the PHY's status registers, including 1000BASE-X. When "CONFIG_PHY_DYNAMIC_ANEG" is not defined, the PHY will advertise hard-coded capabilities, as before.
<snip>
/*****************************************************************************
- Determine the ethernet speed (10/100).
*/
- Determine the ethernet speed (10/100/1000). Return 10 on error.
int miiphy_speed (char *devname, unsigned char addr) {
- unsigned short reg;
- u16 bmcr;
#if defined(CONFIG_PHY_GIGE)
- if (miiphy_read (devname, addr, PHY_1000BTSR, ®)) {
printf ("PHY 1000BT Status read failed\n");- } else {
if (reg != 0xFFFF) {if ((reg & (PHY_1000BTSR_1000FD | PHY_1000BTSR_1000HD)) !=0) {return (_1000BASET);}
- u16 btsr;
+#if defined(CONFIG_PHY_DYNAMIC_ANEG)
- u16 bmsr;
- /* Check for 1000BASE-X. */
- if (miiphy_read (devname, addr, PHY_BMSR, &bmsr)) {
printf ("PHY status");goto miiphy_read_failed;- }
- if (bmsr & PHY_BMSR_EXT_STAT) {
u16 exsr;
Please don't define variables in code blocks. The original code used a single variable called 'reg' that worked well enough. The bitmasks (PHY_BMSR_* and so on) effectively tell the reader what register he/she is looking at.
I'm going to stop here and wait for you to separate content from cosmetic, and hopefully fix up what I've mentioned here.
regards, Ben
Ben Warren wrote:
Larry Johnson wrote:
This patch adds a new switch: "CONFIG_PHY_DYNAMIC_ANEG". When this symbol is defined, the PHY will advertise it's capabilities for autonegotiation based on the capabilities shown in the PHY's status registers, including 1000BASE-X. When "CONFIG_PHY_DYNAMIC_ANEG" is not defined, the PHY will advertise hard-coded capabilities, as before.
<snip> > - > /***************************************************************************** > > * > - * Determine the ethernet speed (10/100). > + * Determine the ethernet speed (10/100/1000). Return 10 on error. > */ > int miiphy_speed (char *devname, unsigned char addr) > { > - unsigned short reg; > + u16 bmcr; > > #if defined(CONFIG_PHY_GIGE) > - if (miiphy_read (devname, addr, PHY_1000BTSR, ®)) { > - printf ("PHY 1000BT Status read failed\n"); > - } else { > - if (reg != 0xFFFF) { > - if ((reg & (PHY_1000BTSR_1000FD | PHY_1000BTSR_1000HD)) > !=0) { > - return (_1000BASET); > - } > + u16 btsr; > + > +#if defined(CONFIG_PHY_DYNAMIC_ANEG) > + u16 bmsr; > + > + /* Check for 1000BASE-X. */ > + if (miiphy_read (devname, addr, PHY_BMSR, &bmsr)) { > + printf ("PHY status"); > + goto miiphy_read_failed; > + } > + if (bmsr & PHY_BMSR_EXT_STAT) { > + u16 exsr; > Please don't define variables in code blocks. The original code used a single variable called 'reg' that worked well enough. The bitmasks (PHY_BMSR_* and so on) effectively tell the reader what register he/she is looking at.
I'm going to stop here and wait for you to separate content from cosmetic, and hopefully fix up what I've mentioned here.
regards, Ben
Hi Ben,
You bring up two issues here, and I'm wondering how you and others feel about them. The first is whether all variables should be declared at the start of the function, or within the smallest block in which they are used. I prefer to declare variables close to where they are used, but don't mind doing it either way.
I do think that using separate dedicated variables for to hold the different registers is a better coding practice. The compiler can do a better job keeping track of variable lifetimes than I can, and someone modifying the code later won't have to check to see if it's safe to modify 'reg' at some point or other. Also, the register and bitmask definitions make it obvious that the code is correct, as in
"if (miiphy_read (devname, addr, PHY_BMSR, &bmsr)) {"
and
"if (bmsr & PHY_BMSR_EXT_STAT) {".
Those are the advantage I see, what do you see as the disadvantages?
Best regards, Larry
participants (5)
-
Ben Warren -
Jon Loeliger -
Larry Johnson -
Stefan Roese -
Wolfgang Denk