Re: [U-Boot-Users] [PATCH 5/5] mpc83xx: UEC: add support for Broadcom BCM5481 Gigabit PHY

On Thu, 2008-01-10 at 15:41 -0500, Ben Warren wrote:

Anton Vorontsov wrote:

...

This patch adds basic support for Broadcom BCM5481 PHY, with the quirk needed for at least MPC8360E-RDK.

Quirk comes from MPC8360E-RDK BSP source, I think author is Peter Barada peterb@logicpd.com, but I'm not sure.

There are no openly available specifications for that PHY.

Signed-off-by: Anton Vorontsov avorontsov@ru.mvista.com

drivers/qe/uec_phy.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++ drivers/qe/uec_phy.h | 5 +++ 2 files changed, 82 insertions(+), 0 deletions(-)

diff --git a/drivers/qe/uec_phy.c b/drivers/qe/uec_phy.c index ca6faa6..6882d03 100644 --- a/drivers/qe/uec_phy.c +++ b/drivers/qe/uec_phy.c @@ -237,6 +237,44 @@ static int gbit_config_aneg (struct uec_mii_info *mii_info) return 0; }

+static int gbit_read_status(struct uec_mii_info *mii_info) +{

• u16 status;
• int err;
• err = genmii_update_link(mii_info);
• if (err)

• return err;
  

• if (mii_info->autoneg) {

• mii_info->pause = 0;
  

• status = phy_read(mii_info, MII_1000BASETSTATUS);
  

• if (status & (LPA_1000FULL | LPA_1000HALF)) {
  

• 	mii_info->speed = SPEED_1000;
  

• 	if (status & LPA_1000FULL)
  

• 		mii_info->duplex = DUPLEX_FULL;
  

• 	else
  

• 		mii_info->duplex = DUPLEX_HALF;
  

• } else {
  

• 	status = phy_read(mii_info, PHY_ANLPAR);
  

• 	if (status & (PHY_ANLPAR_10FD | PHY_ANLPAR_TXFD))
  

• 		mii_info->duplex = DUPLEX_FULL;
  

• 	else
  

• 		mii_info->duplex = DUPLEX_HALF;
  

• 	if (status & (PHY_ANLPAR_TXFD | PHY_ANLPAR_TX))
  

• 		mii_info->speed = SPEED_100;
  

• 	else
  

• 		mii_info->speed = SPEED_10;
  

• }
  

• }
• /*

• * On non-aneg, we assume what we put in BMCR is the speed,
  

• * though magic-aneg shouldn't prevent this case from occurring.
  

• */
  

• return 0;
  

+}

Please roll the 1Gb code into genmii_read_status. 10/100 PHYs should report 0 in the MII_1000BASETSTATUS register so it should work.

...

static int marvell_config_aneg (struct uec_mii_info *mii_info) { /* The Marvell PHY has an errata which requires @@ -319,6 +357,35 @@ static int genmii_read_status (struct uec_mii_info *mii_info) return 0; }

+static int bcm_init(struct uec_mii_info *mii_info) +{

• gbit_config_aneg(mii_info);

+#ifdef CONFIG_MPC8360ERDK

• {

• u16 val;
  

• int cnt = 50;
  

• /* Wait for aneg to complete. */
  

• do
  

• 	val = phy_read(mii_info, PHY_BMSR);
  

• while (--cnt && !(val & PHY_BMSR_AUTN_COMP));
  

• /* Set RDX clk delay. */
  

• phy_write(mii_info, 0x18, 0x7 | (7 << 12));
  

• val = phy_read(mii_info, 0x18);
  

• /* Set RDX-RXC skew. */
  

• val |= (1<<8);
  

• val |= (7 | (7 << 12));
  

• /* Write bits 14:0. */
  

• val |= (1<<15);
  

• phy_write(mii_info, 0x18, val);
  

• }

+#endif

What is it that makes this specific to one board? Magic numbers are baaaad, especially for industry-standard memory maps (I'm thinking of offset 0x18 in case it's not obvious)

Its specific to the board, in that when hooked up via RGMII the setting in the PHY is required to supply adequate delay between the RXD and RXC signals instead of using trace lengths to achieve timing. I know magic numbers are baaaaad, but the PHY spec is only available under NDA and as such descriptions(including informative constants, etc) can not be disclosed. If it wasn't for this one register setting(after reset), the PHY, as used, wouldn't require any PHY-specific code(outside of ID'ng it as a 5481).

...

• return 0;
  

+}

static int marvell_read_status (struct uec_mii_info *mii_info) { u16 status; @@ -491,6 +558,15 @@ static struct phy_info phy_info_marvell = { .config_intr = &marvell_config_intr, };

+static struct phy_info phy_info_bcm5481 = {

• .phy_id = 0x0143bca0,
• .phy_id_mask = 0xffffff0,
• .name = "Broadcom 5481",
• .features = MII_GBIT_FEATURES,
• .read_status = gbit_read_status,
• .init = bcm_init,

+};

static struct phy_info phy_info_genmii = { .phy_id = 0x00000000, .phy_id_mask = 0x00000000, @@ -504,6 +580,7 @@ static struct phy_info *phy_info[] = { &phy_info_dm9161, &phy_info_dm9161a, &phy_info_marvell,

• &phy_info_bcm5481, &phy_info_genmii, NULL

}; diff --git a/drivers/qe/uec_phy.h b/drivers/qe/uec_phy.h index e59a940..6f769fb 100644 --- a/drivers/qe/uec_phy.h +++ b/drivers/qe/uec_phy.h @@ -29,6 +29,11 @@ #define MII_1000BASETCONTROL_FULLDUPLEXCAP 0x0200 #define MII_1000BASETCONTROL_HALFDUPLEXCAP 0x0100

+/* 1000BT status */ +#define MII_1000BASETSTATUS 0x0a +#define LPA_1000FULL 0x0400 +#define LPA_1000HALF 0x0200

/* Cicada Extended Control Register 1 */ #define MII_CIS8201_EXT_CON1 0x17 #define MII_CIS8201_EXTCON1_INIT 0x0000