From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Warren Date: Fri, 11 Jan 2008 00:26:45 -0500 Subject: [U-Boot-Users] [PATCH 5/5] mpc83xx: UEC: add support for Broadcom BCM5481 Gigabit PHY In-Reply-To: <1199999420.12532.79.camel@blackhole> References: <20080109175051.GA18582@localhost.localdomain> <20080109175815.GE25068@localhost.localdomain> <478682FF.1000607@gmail.com> <1199999420.12532.79.camel@blackhole> Message-ID: <4786FE15.8080204@gmail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Peter Barada wrote: > 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 , but I'm not sure. >>> >>> There are no openly available specifications for that PHY. >>> >>> Signed-off-by: Anton Vorontsov >>> --- >>> 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). > > Is this just due to a layout problem on the reference board, or do you always need to do this when connecting this MAC/PHY combo via RGMII? If the former, the additional registers needs to be set in board code. If the latter, please do something like this: Add an 'enet_interface_e if_mode' entry to the 'struct uec_mii_info' definition, and fill it in when you call 'uec_set_mac_if_mode()' and check the value for RGMII when 'bcm_init()' is called. Sorry, but having a *reference* board #ifdef'd in library code is just wrong. Regarding the magic numbers, something like this would be better: #define MII_BCM_BCM_PRIVATE_0 0x16 #define MII_BCM_BCM_PRIVATE_1 0x17 #define MII_BCM_BCM_PRIVATE_2 0x18 regards, Ben