* [U-Boot] [PATCH 1/1] Adding MSCC PHY-VSC8530-VSC8531-VSC8540-VSC8541
2016-12-07 17:11 [U-Boot] [PATCH 1/1] Adding MSCC PHY-VSC8530-VSC8531-VSC8540-VSC8541 John Haechten
@ 2016-12-08 12:24 ` Alexey Brodkin
2016-12-08 17:04 ` Joe Hershberger
2016-12-08 19:02 ` Joe Hershberger
1 sibling, 1 reply; 6+ messages in thread
From: Alexey Brodkin @ 2016-12-08 12:24 UTC (permalink / raw)
To: u-boot
Hi John,
First of all I'm wondering if there was a real need to add that many people in Cc?
Normally sybsystem maintainer and active contributors to code you modified are
good options here.
Then have you run your patch through checkpatch?
Obviously it was not the case because it says me:
-------------------------->8---------------------
total: 570 errors, 251 warnings, 0 checks, 1170 lines checked
-------------------------->8---------------------
Please fix all that first. That's way too many errors for such a small patch!
Take a look at this article -?http://www.denx.de/wiki/view/U-Boot/Patches.
Still some comments below.
On Wed, 2016-12-07 at 17:11 +0000, John Haechten wrote:
> From 127c5fb9ef390cad5cf58e446110a696cf111345 Mon Sep 17 00:00:00 2001
> From: John Haechten <john.haechten@microsemi.com>
> Date: Wed, 7 Dec 2016 07:51:37 -0800
> Subject: [PATCH] Adding MSCC PHY-VSC8530-VSC8531-VSC8540-VSC8541
Commit title is not very readable.
Remember this title is what people will see later on while looking at git log.
So maybe something like "net/phy: Add Microsemi vsc85xx PHYs"?
> (C) Copyright 2016 Microsemi Corporation, MIT License (MIT)
Why do you need that in commit message?
> Author:John Haechten <john.haechten@microsemi.com>
Shouldn't it be just "Signed-off-by" instead (already existed below)?
> Reviewed-by:Howard Hicks <howard.hicks@microsemi.com>
> Reviewed-by:Joe Hershberger <joe.hershberger@ni.com>
> Tested-by:Howard Hicks <howard.hicks@microsemi.com>
> Version:1
What's this?
> Prefix:None
Ditto.
> Signed-off-by: John Haechten <john.haechten@microsemi.com>
>
> ---
>
> drivers/net/phy/Makefile??????????? |?? 1 +
> drivers/net/phy/mscc.c????????????? | 480 ++++++++++++++++++++++++++++++++++++
> drivers/net/phy/phy.c?????????????? |?? 3 +
> include/config_phylib_all_drivers.h |?? 1 +
> include/configs/am335x_evm.h??????? |?? 3 +
> include/phy.h?????????????????????? |?? 1 +
> 6 files changed, 489 insertions(+)
> create mode 100644 drivers/net/phy/mscc.c
>
> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> index 1e299b9..d372971 100644
> --- a/drivers/net/phy/Makefile
> +++ b/drivers/net/phy/Makefile
> @@ -27,3 +27,4 @@ obj-$(CONFIG_PHY_TERANETICS) += teranetics.o
> obj-$(CONFIG_PHY_TI) += ti.o
> obj-$(CONFIG_PHY_XILINX) += xilinx_phy.o
> obj-$(CONFIG_PHY_VITESSE) += vitesse.o
> +obj-$(CONFIG_PHY_MSCC) += mscc.o
> diff --git a/drivers/net/phy/mscc.c b/drivers/net/phy/mscc.c
> new file mode 100644
> index 0000000..e63bd43
> --- /dev/null
> +++ b/drivers/net/phy/mscc.c
> @@ -0,0 +1,480 @@
> +/*
> + * Driver for Microsemi VSC85xx PHYs
> + *
> + * Author: John Haechten
> + *
> + * The MIT License (MIT)
> + *
> + * Copyright (c) 2016 Microsemi Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
Please check?http://www.denx.de/wiki/view/U-Boot/Patches#Attributing_Code_Copyrights_Sign
> + */
> +
> +#include <miiphy.h>
> +#include <bitfield.h>
Maybe swap includes so they are sorted alphabetically or this order is really required?
> +
> +/* Note: To include this PHY Driver file, #define CONFIG_PHY_MSCC */
IMHO that note should be removed. Even though it might not be super obvious but that's
how legacy configuration is done in U-Boot. Moreover I would strongly recommend to use Kconfig
stuff for new drivers instead of legacy approach with includes.
> +/* Microsemi PHY ID's */
> +#define PHY_ID_VSC8530??????????????? ??0x00070560
> +#define PHY_ID_VSC8531????????????????? 0x00070570
> +#define PHY_ID_VSC8540????????????????? 0x00070760
> +#define PHY_ID_VSC8541????????????????? 0x00070770
Maybe add a newline here?
> +/* Microsemi VSC85xx PHY Register Pages */
> +#define MSCC_EXT_PAGE_ACCESS??????????? 31???? /* Page Access Register */
> +#define MSCC_PHY_PAGE_STD????????? 0x0000 /* Standard registers */
> +#define MSCC_PHY_PAGE_EXT1???????? 0x0001 /* Extended registers - page 1 */
> +#define MSCC_PHY_PAGE_EXT2???????? 0x0002 /* Extended registers - page 2 */
> +#define MSCC_PHY_PAGE_EXT3???????? 0x0003 /* Extended registers - page 3 */
> +#define MSCC_PHY_PAGE_EXT4???????? 0x0004 /* Extended registers - page 4 */
> +#define MSCC_PHY_PAGE_GPIO???????? 0x0010 /* GPIO registers */
> +#define MSCC_PHY_PAGE_TEST???????? 0x2A30 /* TEST Page registers */
> +#define MSCC_PHY_PAGE_TR?????????? 0x52B5 /* Token Ring Page registers */
Ditto.
> +/* Std Page Register 28 - PHY AUX Control/Status */
> +#define MIIM_AUX_CNTRL_STAT_REG????????? 0x1c
> +#define MIIM_AUX_CNTRL_STAT_ACTIPHY_TO?? 0x0004
> +#define MIIM_AUX_CNTRL_STAT_F_DUPLEX???? 0x0020
> +#define MIIM_AUX_CNTRL_STAT_SPEED_MASK?? 0x0018
> +#define MIIM_AUX_CNTRL_STAT_SPEED_POS??? (3)
> +#define MIIM_AUX_CNTRL_STAT_SPEED_10M??? (0x0)
> +#define MIIM_AUX_CNTRL_STAT_SPEED_100M?? (0x1)
> +#define MIIM_AUX_CNTRL_STAT_SPEED_1000M? (0x2)
> +/* Std Page Register 23 - Extended PHY CTRL_1 */
> +#define MSCC_PHY_EXT_PHY_CNTL_1???????? 23
> +#define MAC_IF_SELECTION_MASK?????????? 0x1800
> +#define MAC_IF_SELECTION_GMII?????????? 0
> +#define MAC_IF_SELECTION_RMII?????????? 1
> +#define MAC_IF_SELECTION_RGMII????????? 2
> +#define MAC_IF_SELECTION_POS?????????? ?11
> +#define MAC_IF_SELECTION_WIDTH????????? 2
> +/* Extended Page 2 Register 20E2 */
> +#define MSCC_PHY_RGMII_CNTL???????????? 20
> +#define VSC_FAST_LINK_FAIL2_ENA_MASK??? 0x8000
> +#define RX_CLK_OUT_MASK???????????????? 0x0800
> +#define RX_CLK_OUT_POS???????? ?????????11
> +#define RX_CLK_OUT_WIDTH??????????????? 1
> +#define RX_CLK_OUT_NORMAL?????????????? 0
> +#define RX_CLK_OUT_DISABLE????????????? 1
> +#define RGMII_RX_CLK_DELAY_POS????????? 4
> +#define RGMII_RX_CLK_DELAY_WIDTH??????? 3
> +#define RGMII_RX_CLK_DELAY_MASK???????? 0x0070
> +#define RGMII_TX_CLK_DELAY_POS????????? 0
> +#define RGMII_TX_CLK_DELAY_WIDTH??????? 3
> +#define RGMII_TX_CLK_DELAY_MASK???????? 0x0007
> +/* Extended Page 2 Register 27E2 */
> +#define MSCC_PHY_WOL_MAC_CONTROL??????? 27
> +#define EDGE_RATE_CNTL_POS????????????? 5
> +#define EDGE_RATE_CNTL_WIDTH??????????? 3
> +#define EDGE_RATE_CNTL_MASK???????????? 0x00E0
> +#define RMII_CLK_OUT_ENABLE_POS???????? 4
> +#define RMII_CLK_OUT_ENABLE_WIDTH?????? 1
> +#define RMII_CLK_OUT_ENABLE_MASK??????? 0x10
> +/* Token Ring Page 0x52B5 Registers */
> +#define MSCC_PHY_REG_TR_ADDR_16????????? 16
> +#define MSCC_PHY_REG_TR_DATA_17????????? 17
> +#define MSCC_PHY_REG_TR_DATA_18????????? 18
> +/* Token Ring Registers */
> +#define MSCC_PHY_TR_LINKDETCTRL_POS????? 3
> +#define MSCC_PHY_TR_LINKDETCTRL_WIDTH??? 2
> +#define MSCC_PHY_TR_LINKDETCTRL_MASK???? 0x18
> +#define MSCC_PHY_TR_VGATHRESH100_POS???? 0
> +#define MSCC_PHY_TR_VGATHRESH100_WIDTH?? 7
> +#define MSCC_PHY_TR_VGATHRESH100_MASK??? 0x7f
> +#define MSCC_PHY_TR_VGAGAIN10_U_POS????? 0
> +#define MSCC_PHY_TR_VGAGAIN10_U_WIDTH??? 1
> +#define MSCC_PHY_TR_VGAGAIN10_U_MASK???? 0x01
> +#define MSCC_PHY_TR_VGAGAIN10_L_POS????? 12
> +#define MSCC_PHY_TR_VGAGAIN10_L_WIDTH??? 4
> +#define MSCC_PHY_TR_VGAGAIN10_L_MASK???? 0xf000
> +/* General Timeout Values */
> +#define MSCC_PHY_RESET_TIMEOUT?????????? (100)
> +#define MSCC_PHY_MICRO_TIMEOUT?????????? (500)
> +
> +/**< RGMII/GMII Clock Delay (Skew) Options */
Very strange comment like "/**<".
> +enum vsc_phy_rgmii_skew {
> +???? VSC_PHY_RGMII_DELAY_200_PS,
> +???? VSC_PHY_RGMII_DELAY_800_PS,
> +???? VSC_PHY_RGMII_DELAY_1100_PS,
> +???? VSC_PHY_RGMII_DELAY_1700_PS,
> +???? VSC_PHY_RGMII_DELAY_2000_PS,
> +???? VSC_PHY_RGMII_DELAY_2300_PS,
> +???? VSC_PHY_RGMII_DELAY_2600_PS,
> +???? VSC_PHY_RGMII_DELAY_3400_PS
That's a good practice to add comma even after the last member of enum.
This makes the next patch much cleaner if you want to add another value to this enum.
> +};
> +
> +/**< MAC i/f Clock Edge Rage Control (Slew), See Reg27E2? */
Again strange comment.
> +enum vsc_phy_clk_slew {
> +???? VSC_PHY_CLK_SLEW_RATE_0,
> +???? VSC_PHY_CLK_SLEW_RATE_1,
> +???? VSC_PHY_CLK_SLEW_RATE_2,
> +???? VSC_PHY_CLK_SLEW_RATE_3,
> +???? VSC_PHY_CLK_SLEW_RATE_4,
> +???? VSC_PHY_CLK_SLEW_RATE_5,
> +???? VSC_PHY_CLK_SLEW_RATE_6,
> +???? VSC_PHY_CLK_SLEW_RATE_7
> +};
> +
> +
> +static int mscc_vsc8531_vsc8541_init_scripts(struct phy_device *phydev)
> +{
> +???? u16 reg_val17;
> +???? u16 reg_val18;
Any reason to keep variables in different lines?
> +
> +???? /* Set to Access Token Ring Registers */
> +???? phy_write(phydev, MDIO_DEVAD_NONE,
> +?????????? ? MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_TR);
> +
> +???? /* Update LinkDetectCtrl default to optimized values */
> +???? /* Determined during Silicon Validation Testing */
> +???? phy_write(phydev, MDIO_DEVAD_NONE, MSCC_PHY_REG_TR_ADDR_16, 0xA7F8);
Cryptic values are not very nice to deal with. I'd advise you to use defines here.
Moreover I'm wondering:
?a) why all this complicated setup is required?
?b) is it mentioned in any datasheet? If it is mentioned it worth adding a comment
? ? about source of those figures and explanation of the procedure itself.
? ? This will help mere mortals to figure out what is done here and if required people will
? ? use that knowledge as a reference in other projects.
> +???? reg_val17 = phy_read(phydev, MDIO_DEVAD_NONE, MSCC_PHY_REG_TR_DATA_17);
> +???? reg_val17 = bitfield_replace(reg_val17, MSCC_PHY_TR_LINKDETCTRL_POS,
> +?????????????????????? ???? MSCC_PHY_TR_LINKDETCTRL_WIDTH, 3);
> +???? phy_write(phydev, MDIO_DEVAD_NONE, MSCC_PHY_REG_TR_DATA_17, reg_val17);
> +???? phy_write(phydev, MDIO_DEVAD_NONE, MSCC_PHY_REG_TR_ADDR_16, 0x87F8);
> +
> +???? /* Update VgaThresh100 defaults to optimized values */
> +???? /* Determined during Silicon Validation Testing */
> +???? phy_write(phydev, MDIO_DEVAD_NONE, MSCC_PHY_REG_TR_ADDR_16, 0xAFA4);
> +???? reg_val18 = phy_read(phydev, MDIO_DEVAD_NONE, MSCC_PHY_REG_TR_DATA_18);
> +???? reg_val18 = bitfield_replace(reg_val18, MSCC_PHY_TR_VGATHRESH100_POS,
> +?????????????????????? ???? MSCC_PHY_TR_VGATHRESH100_WIDTH, 24);
> +???? phy_write(phydev, MDIO_DEVAD_NONE, MSCC_PHY_REG_TR_DATA_18, reg_val18);
> +???? phy_write(phydev, MDIO_DEVAD_NONE, MSCC_PHY_REG_TR_ADDR_16, 0x8FA4);
> +
> +???? /* Update VgaGain10 defaults to optimized values */
> +???? /* Determined during Silicon Validation Testing */
> +???? phy_write(phydev, MDIO_DEVAD_NONE, MSCC_PHY_REG_TR_ADDR_16, 0xAF92);
> +???? reg_val18 = phy_read(phydev, MDIO_DEVAD_NONE, MSCC_PHY_REG_TR_DATA_18);
> +???? reg_val17 = phy_read(phydev, MDIO_DEVAD_NONE, MSCC_PHY_REG_TR_DATA_17);
> +???? reg_val18 = bitfield_replace(reg_val18, MSCC_PHY_TR_VGAGAIN10_U_POS,
> +?????????????????????? ???? MSCC_PHY_TR_VGAGAIN10_U_WIDTH, 0);
> +
> +???? reg_val17 = bitfield_replace(reg_val17, MSCC_PHY_TR_VGAGAIN10_L_POS,
> +?????????????????????? ???? MSCC_PHY_TR_VGAGAIN10_L_WIDTH, 1);
> +???? phy_write(phydev, MDIO_DEVAD_NONE, MSCC_PHY_REG_TR_DATA_18, reg_val18);
> +???? phy_write(phydev, MDIO_DEVAD_NONE, MSCC_PHY_REG_TR_DATA_17, reg_val17);
> +???? phy_write(phydev, MDIO_DEVAD_NONE, MSCC_PHY_REG_TR_ADDR_16, 0x8F92);
> +
> +???? /* Set back to Access Standard Page Registers */
> +???? phy_write(phydev, MDIO_DEVAD_NONE, MSCC_EXT_PAGE_ACCESS,
> +?????????? ? MSCC_PHY_PAGE_STD);
> +
> +???? return 0;
> +}
> +
> +static int mscc_parse_status(struct phy_device *phydev)
> +{
> +???? u16 speed;
> +???? u16 mii_reg;
> +
> +???? mii_reg = phy_read(phydev, MDIO_DEVAD_NONE, MIIM_AUX_CNTRL_STAT_REG);
u16 speed, mii_reg =?phy_read(phydev, MDIO_DEVAD_NONE, MIIM_AUX_CNTRL_STAT_REG);
> +???? if (mii_reg & MIIM_AUX_CNTRL_STAT_F_DUPLEX)
> +?????????? phydev->duplex = DUPLEX_FULL;
> +???? else
> +?????????? phydev->duplex = DUPLEX_HALF;
> +
> +???? speed = mii_reg & MIIM_AUX_CNTRL_STAT_SPEED_MASK;
> +???? speed = speed >> MIIM_AUX_CNTRL_STAT_SPEED_POS;
> +
> +???? switch (speed) {
> +???? case MIIM_AUX_CNTRL_STAT_SPEED_1000M:
> +?????????? phydev->speed = SPEED_1000;
> +?????????? break;
> +???? case MIIM_AUX_CNTRL_STAT_SPEED_100M:
> +?????????? phydev->speed = SPEED_100;
> +?????????? break;
> +???? case MIIM_AUX_CNTRL_STAT_SPEED_10M:
> +?????????? phydev->speed = SPEED_10;
> +?????????? break;
> +???? default:
> +?????????? phydev->speed = SPEED_10;
> +?????????? break;
> +???? }
> +
> +???? return 0;
> +}
> +
> +static int mscc_startup(struct phy_device *phydev)
> +{
> +???? int ret;
> +
> +???? ret = genphy_update_link(phydev);
Merge two lines above together?
> +???? if (ret)
> +?????????? return ret;
Newline?
> +???? return mscc_parse_status(phydev);
> +}
> +
> +static int mscc_phy_soft_reset(struct phy_device *phydev)
> +{
> +???? int???? rc = 0;
> +???? u16???? timeout = MSCC_PHY_RESET_TIMEOUT;
> +???? u16???? reg_val = 0;
> +
> +???? phy_write(phydev, MDIO_DEVAD_NONE, MSCC_EXT_PAGE_ACCESS,
> +?????????? ? MSCC_PHY_PAGE_STD);
> +
> +???? reg_val = phy_read(phydev, MDIO_DEVAD_NONE, MII_BMCR);
> +???? reg_val |= BMCR_RESET;
> +???? phy_write(phydev, MDIO_DEVAD_NONE, MII_BMCR, reg_val);
I'd merge two lines above, i.e. move "|= BMCR_RESET" right in?phy_write().
> +???? reg_val = phy_read(phydev, MDIO_DEVAD_NONE, MII_BMCR);
> +
> +???? while ((reg_val & BMCR_RESET) && (timeout > 0)) {
> +?????????? reg_val = phy_read(phydev, MDIO_DEVAD_NONE, MII_BMCR);
if (!(reg_val & BMCR_RESET))
return 0;
> +?????????? timeout--;
> +?????????? udelay(1000);?? /* 1 ms */
> +???? }
If above is done then no need to check timeout here, just bail on timeout here.
And BTW "rc" might be removed than as well.
> +???? if (timeout == 0) {
> +?????????? printf("MSCC PHY Soft_Reset Error: mac i/f = 0x%x\n",
> +?????????? ?????? phydev->interface);
> +?????????? rc = -ETIME;
> +???? }
> +
> +???? return rc;
> +}
> +
> +static int vsc8531_vsc8541_mac_config(struct phy_device *phydev)
> +{
> +???? int?? rc = 0;
> +???? u16???? reg_val = 0;
> +???? u16???? mac_if = 0;
> +???? u16???? rx_clk_out = 0;
Different spacing?
> +
> +???? /* For VSC8530/31 the only MAC modes are RMII/RGMII. */
> +???? /* For VSC8540/41 the only MAC modes are (G)MII and RMII/RGMII. */
> +???? /* Setup MAC Configuration */
> +???? switch (phydev->interface) {
> +???? case PHY_INTERFACE_MODE_MII:
> +???? case PHY_INTERFACE_MODE_GMII:
> +?????????? /* Set Reg23.12:11=0 */
> +?????????? mac_if = MAC_IF_SELECTION_GMII;
> +?????????? /* Set Reg20E2.11=1 */
> +?????????? rx_clk_out = RX_CLK_OUT_DISABLE;
> +?????????? break;
> +
> +???? case PHY_INTERFACE_MODE_RMII:
> +?????????? /* Set Reg23.12:11=1 */
> +?????????? mac_if = MAC_IF_SELECTION_RMII;
> +?????????? /* Set Reg20E2.11=0 */
> +?????????? rx_clk_out = RX_CLK_OUT_NORMAL;
> +?????????? break;
> +
> +???? case PHY_INTERFACE_MODE_RGMII:
> +?????????? /* Set Reg23.12:11=2 */
> +?????????? mac_if = MAC_IF_SELECTION_RGMII;
> +?????????? /* Set Reg20E2.11=0 */
> +?????????? rx_clk_out = RX_CLK_OUT_NORMAL;
> +?????????? break;
> +
> +???? default:
> +?????????? printf("MSCC PHY - INVALID MAC i/f Config: mac i/f = 0x%x\n",
> +?????????? ?????? phydev->interface);
> +?????????? return -EINVAL;
> +???? }
> +
> +???? phy_write(phydev, MDIO_DEVAD_NONE, MSCC_EXT_PAGE_ACCESS,
> +?????????? ? MSCC_PHY_PAGE_STD);
> +
> +???? /* Read Reg23 */
Completely senseless comment. Code below makes much more sense.
> +???? reg_val = phy_read(phydev, MDIO_DEVAD_NONE,
> +???????????????? ?? MSCC_PHY_EXT_PHY_CNTL_1);
> +???? /* Set MAC i/f bits Reg23.12:11 */
> +???? reg_val = bitfield_replace(reg_val, MAC_IF_SELECTION_POS,
> +?????????????????????? ?? MAC_IF_SELECTION_WIDTH, mac_if);
> +???? /* Update Reg23.12:11 */
> +???? phy_write(phydev, MDIO_DEVAD_NONE,
> +?????????? ? MSCC_PHY_EXT_PHY_CNTL_1, reg_val);
> +???? /* Setup ExtPg_2 Register Access */
> +???? phy_write(phydev, MDIO_DEVAD_NONE,
> +?????????? ? MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_EXT2);
> +???? /* Read Reg20E2 */
> +???? reg_val = phy_read(phydev, MDIO_DEVAD_NONE,
> +???????????????? ?? MSCC_PHY_RGMII_CNTL);
> +???? reg_val = bitfield_replace(reg_val, RX_CLK_OUT_POS,
> +?????????????????????? ?? RX_CLK_OUT_WIDTH, rx_clk_out);
> +???? /* Update Reg20E2.11 */
> +???? phy_write(phydev, MDIO_DEVAD_NONE,
> +?????????? ? MSCC_PHY_RGMII_CNTL, reg_val);
> +???? /* Before leaving - Change back to Std Page Register Access */
> +???? phy_write(phydev, MDIO_DEVAD_NONE, MSCC_EXT_PAGE_ACCESS,
> +?????????? ? MSCC_PHY_PAGE_STD);
> +
> +???? return rc;
> +}
> +
-Alexey
^ permalink raw reply [flat|nested] 6+ messages in thread* [U-Boot] [PATCH 1/1] Adding MSCC PHY-VSC8530-VSC8531-VSC8540-VSC8541
2016-12-07 17:11 [U-Boot] [PATCH 1/1] Adding MSCC PHY-VSC8530-VSC8531-VSC8540-VSC8541 John Haechten
2016-12-08 12:24 ` Alexey Brodkin
@ 2016-12-08 19:02 ` Joe Hershberger
1 sibling, 0 replies; 6+ messages in thread
From: Joe Hershberger @ 2016-12-08 19:02 UTC (permalink / raw)
To: u-boot
Hi John,
I have some additional comments beyond Alexey's.
On Wed, Dec 7, 2016 at 11:11 AM, John Haechten
<john.haechten@microsemi.com> wrote:
> From 127c5fb9ef390cad5cf58e446110a696cf111345 Mon Sep 17 00:00:00 2001
> From: John Haechten <john.haechten@microsemi.com>
> Date: Wed, 7 Dec 2016 07:51:37 -0800
> Subject: [PATCH] Adding MSCC PHY-VSC8530-VSC8531-VSC8540-VSC8541
>
> (C) Copyright 2016 Microsemi Corporation, MIT License (MIT)
> Author:John Haechten <john.haechten@microsemi.com>
> Reviewed-by:Howard Hicks <howard.hicks@microsemi.com>
> Reviewed-by:Joe Hershberger <joe.hershberger@ni.com>
You should not add a "Reviewed-by:" tag just because I gave feedback.
The meaning of a Reviewed-by: tag is that it is reviewed and
acceptable in the eyes of that reviewer. The reviewer will reply with
the Reviewed-by: tag when they approve. If they reply to your patch
and there is no need for a new version, then there is nothing further
that you need to do. If you do get a Reviewed-by: that someone
replies, but some other reviewer or custodian asks for a change that
means a new version, then you need to add any Reviewed-by: or
Tested-by: tags that people sent (unless the change is so drastic as
to invalidate the review or test).
Also, all tags should be followed by a space before the name of the
person or other content. Perhaps this is why patman isn't handling
your tags properly.
> Tested-by:Howard Hicks <howard.hicks@microsemi.com>
> Version:1
In the future, you should not specify a version until it is version 2.
> Prefix:None
> Signed-off-by: John Haechten <john.haechten@microsemi.com>
>
> ---
>
> drivers/net/phy/Makefile | 1 +
> drivers/net/phy/mscc.c | 480 ++++++++++++++++++++++++++++++++++++
> drivers/net/phy/phy.c | 3 +
> include/config_phylib_all_drivers.h | 1 +
> include/configs/am335x_evm.h | 3 +
> include/phy.h | 1 +
> 6 files changed, 489 insertions(+)
> create mode 100644 drivers/net/phy/mscc.c
>
> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> index 1e299b9..d372971 100644
> --- a/drivers/net/phy/Makefile
> +++ b/drivers/net/phy/Makefile
> @@ -27,3 +27,4 @@ obj-$(CONFIG_PHY_TERANETICS) += teranetics.o
> obj-$(CONFIG_PHY_TI) += ti.o
> obj-$(CONFIG_PHY_XILINX) += xilinx_phy.o
> obj-$(CONFIG_PHY_VITESSE) += vitesse.o
> +obj-$(CONFIG_PHY_MSCC) += mscc.o
> diff --git a/drivers/net/phy/mscc.c b/drivers/net/phy/mscc.c
> new file mode 100644
> index 0000000..e63bd43
> --- /dev/null
> +++ b/drivers/net/phy/mscc.c
> @@ -0,0 +1,480 @@
> +/*
> + * Driver for Microsemi VSC85xx PHYs
> + *
> + * Author: John Haechten
> + *
> + * The MIT License (MIT)
> + *
> + * Copyright (c) 2016 Microsemi Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + *
> + */
> +
> +#include <miiphy.h>
> +#include <bitfield.h>
> +
> +/* Note: To include this PHY Driver file, #define CONFIG_PHY_MSCC */
> +
> +/* Microsemi PHY ID's */
> +#define PHY_ID_VSC8530 0x00070560
> +#define PHY_ID_VSC8531 0x00070570
> +#define PHY_ID_VSC8540 0x00070760
> +#define PHY_ID_VSC8541 0x00070770
> +/* Microsemi VSC85xx PHY Register Pages */
> +#define MSCC_EXT_PAGE_ACCESS 31 /* Page Access Register */
> +#define MSCC_PHY_PAGE_STD 0x0000 /* Standard registers */
> +#define MSCC_PHY_PAGE_EXT1 0x0001 /* Extended registers - page 1 */
> +#define MSCC_PHY_PAGE_EXT2 0x0002 /* Extended registers - page 2 */
> +#define MSCC_PHY_PAGE_EXT3 0x0003 /* Extended registers - page 3 */
> +#define MSCC_PHY_PAGE_EXT4 0x0004 /* Extended registers - page 4 */
> +#define MSCC_PHY_PAGE_GPIO 0x0010 /* GPIO registers */
> +#define MSCC_PHY_PAGE_TEST 0x2A30 /* TEST Page registers */
> +#define MSCC_PHY_PAGE_TR 0x52B5 /* Token Ring Page registers */
> +/* Std Page Register 28 - PHY AUX Control/Status */
> +#define MIIM_AUX_CNTRL_STAT_REG 0x1c
> +#define MIIM_AUX_CNTRL_STAT_ACTIPHY_TO 0x0004
> +#define MIIM_AUX_CNTRL_STAT_F_DUPLEX 0x0020
> +#define MIIM_AUX_CNTRL_STAT_SPEED_MASK 0x0018
> +#define MIIM_AUX_CNTRL_STAT_SPEED_POS (3)
> +#define MIIM_AUX_CNTRL_STAT_SPEED_10M (0x0)
> +#define MIIM_AUX_CNTRL_STAT_SPEED_100M (0x1)
> +#define MIIM_AUX_CNTRL_STAT_SPEED_1000M (0x2)
> +/* Std Page Register 23 - Extended PHY CTRL_1 */
> +#define MSCC_PHY_EXT_PHY_CNTL_1 23
> +#define MAC_IF_SELECTION_MASK 0x1800
> +#define MAC_IF_SELECTION_GMII 0
> +#define MAC_IF_SELECTION_RMII 1
> +#define MAC_IF_SELECTION_RGMII 2
> +#define MAC_IF_SELECTION_POS 11
> +#define MAC_IF_SELECTION_WIDTH 2
> +/* Extended Page 2 Register 20E2 */
> +#define MSCC_PHY_RGMII_CNTL 20
> +#define VSC_FAST_LINK_FAIL2_ENA_MASK 0x8000
> +#define RX_CLK_OUT_MASK 0x0800
> +#define RX_CLK_OUT_POS 11
> +#define RX_CLK_OUT_WIDTH 1
> +#define RX_CLK_OUT_NORMAL 0
> +#define RX_CLK_OUT_DISABLE 1
> +#define RGMII_RX_CLK_DELAY_POS 4
> +#define RGMII_RX_CLK_DELAY_WIDTH 3
> +#define RGMII_RX_CLK_DELAY_MASK 0x0070
> +#define RGMII_TX_CLK_DELAY_POS 0
> +#define RGMII_TX_CLK_DELAY_WIDTH 3
> +#define RGMII_TX_CLK_DELAY_MASK 0x0007
> +/* Extended Page 2 Register 27E2 */
> +#define MSCC_PHY_WOL_MAC_CONTROL 27
> +#define EDGE_RATE_CNTL_POS 5
> +#define EDGE_RATE_CNTL_WIDTH 3
> +#define EDGE_RATE_CNTL_MASK 0x00E0
> +#define RMII_CLK_OUT_ENABLE_POS 4
> +#define RMII_CLK_OUT_ENABLE_WIDTH 1
> +#define RMII_CLK_OUT_ENABLE_MASK 0x10
> +/* Token Ring Page 0x52B5 Registers */
> +#define MSCC_PHY_REG_TR_ADDR_16 16
> +#define MSCC_PHY_REG_TR_DATA_17 17
> +#define MSCC_PHY_REG_TR_DATA_18 18
> +/* Token Ring Registers */
> +#define MSCC_PHY_TR_LINKDETCTRL_POS 3
> +#define MSCC_PHY_TR_LINKDETCTRL_WIDTH 2
> +#define MSCC_PHY_TR_LINKDETCTRL_MASK 0x18
> +#define MSCC_PHY_TR_VGATHRESH100_POS 0
> +#define MSCC_PHY_TR_VGATHRESH100_WIDTH 7
> +#define MSCC_PHY_TR_VGATHRESH100_MASK 0x7f
> +#define MSCC_PHY_TR_VGAGAIN10_U_POS 0
> +#define MSCC_PHY_TR_VGAGAIN10_U_WIDTH 1
> +#define MSCC_PHY_TR_VGAGAIN10_U_MASK 0x01
> +#define MSCC_PHY_TR_VGAGAIN10_L_POS 12
> +#define MSCC_PHY_TR_VGAGAIN10_L_WIDTH 4
> +#define MSCC_PHY_TR_VGAGAIN10_L_MASK 0xf000
> +/* General Timeout Values */
> +#define MSCC_PHY_RESET_TIMEOUT (100)
> +#define MSCC_PHY_MICRO_TIMEOUT (500)
> +
> +/**< RGMII/GMII Clock Delay (Skew) Options */
> +enum vsc_phy_rgmii_skew {
> + VSC_PHY_RGMII_DELAY_200_PS,
> + VSC_PHY_RGMII_DELAY_800_PS,
> + VSC_PHY_RGMII_DELAY_1100_PS,
> + VSC_PHY_RGMII_DELAY_1700_PS,
> + VSC_PHY_RGMII_DELAY_2000_PS,
> + VSC_PHY_RGMII_DELAY_2300_PS,
> + VSC_PHY_RGMII_DELAY_2600_PS,
> + VSC_PHY_RGMII_DELAY_3400_PS
> +};
> +
> +/**< MAC i/f Clock Edge Rage Control (Slew), See Reg27E2 */
> +enum vsc_phy_clk_slew {
> + VSC_PHY_CLK_SLEW_RATE_0,
> + VSC_PHY_CLK_SLEW_RATE_1,
> + VSC_PHY_CLK_SLEW_RATE_2,
> + VSC_PHY_CLK_SLEW_RATE_3,
> + VSC_PHY_CLK_SLEW_RATE_4,
> + VSC_PHY_CLK_SLEW_RATE_5,
> + VSC_PHY_CLK_SLEW_RATE_6,
> + VSC_PHY_CLK_SLEW_RATE_7
> +};
> +
> +
> +static int mscc_vsc8531_vsc8541_init_scripts(struct phy_device *phydev)
> +{
> + u16 reg_val17;
> + u16 reg_val18;
> +
> + /* Set to Access Token Ring Registers */
> + phy_write(phydev, MDIO_DEVAD_NONE,
> + MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_TR);
> +
> + /* Update LinkDetectCtrl default to optimized values */
> + /* Determined during Silicon Validation Testing */
> + phy_write(phydev, MDIO_DEVAD_NONE, MSCC_PHY_REG_TR_ADDR_16, 0xA7F8);
> + reg_val17 = phy_read(phydev, MDIO_DEVAD_NONE, MSCC_PHY_REG_TR_DATA_17);
> + reg_val17 = bitfield_replace(reg_val17, MSCC_PHY_TR_LINKDETCTRL_POS,
> + MSCC_PHY_TR_LINKDETCTRL_WIDTH, 3);
> + phy_write(phydev, MDIO_DEVAD_NONE, MSCC_PHY_REG_TR_DATA_17, reg_val17);
> + phy_write(phydev, MDIO_DEVAD_NONE, MSCC_PHY_REG_TR_ADDR_16, 0x87F8);
> +
> + /* Update VgaThresh100 defaults to optimized values */
> + /* Determined during Silicon Validation Testing */
> + phy_write(phydev, MDIO_DEVAD_NONE, MSCC_PHY_REG_TR_ADDR_16, 0xAFA4);
> + reg_val18 = phy_read(phydev, MDIO_DEVAD_NONE, MSCC_PHY_REG_TR_DATA_18);
> + reg_val18 = bitfield_replace(reg_val18, MSCC_PHY_TR_VGATHRESH100_POS,
> + MSCC_PHY_TR_VGATHRESH100_WIDTH, 24);
> + phy_write(phydev, MDIO_DEVAD_NONE, MSCC_PHY_REG_TR_DATA_18, reg_val18);
> + phy_write(phydev, MDIO_DEVAD_NONE, MSCC_PHY_REG_TR_ADDR_16, 0x8FA4);
> +
> + /* Update VgaGain10 defaults to optimized values */
> + /* Determined during Silicon Validation Testing */
> + phy_write(phydev, MDIO_DEVAD_NONE, MSCC_PHY_REG_TR_ADDR_16, 0xAF92);
> + reg_val18 = phy_read(phydev, MDIO_DEVAD_NONE, MSCC_PHY_REG_TR_DATA_18);
> + reg_val17 = phy_read(phydev, MDIO_DEVAD_NONE, MSCC_PHY_REG_TR_DATA_17);
> + reg_val18 = bitfield_replace(reg_val18, MSCC_PHY_TR_VGAGAIN10_U_POS,
> + MSCC_PHY_TR_VGAGAIN10_U_WIDTH, 0);
> +
> + reg_val17 = bitfield_replace(reg_val17, MSCC_PHY_TR_VGAGAIN10_L_POS,
> + MSCC_PHY_TR_VGAGAIN10_L_WIDTH, 1);
> + phy_write(phydev, MDIO_DEVAD_NONE, MSCC_PHY_REG_TR_DATA_18, reg_val18);
> + phy_write(phydev, MDIO_DEVAD_NONE, MSCC_PHY_REG_TR_DATA_17, reg_val17);
> + phy_write(phydev, MDIO_DEVAD_NONE, MSCC_PHY_REG_TR_ADDR_16, 0x8F92);
> +
> + /* Set back to Access Standard Page Registers */
> + phy_write(phydev, MDIO_DEVAD_NONE, MSCC_EXT_PAGE_ACCESS,
> + MSCC_PHY_PAGE_STD);
> +
> + return 0;
> +}
> +
> +static int mscc_parse_status(struct phy_device *phydev)
> +{
> + u16 speed;
> + u16 mii_reg;
> +
> + mii_reg = phy_read(phydev, MDIO_DEVAD_NONE, MIIM_AUX_CNTRL_STAT_REG);
> +
> + if (mii_reg & MIIM_AUX_CNTRL_STAT_F_DUPLEX)
> + phydev->duplex = DUPLEX_FULL;
> + else
> + phydev->duplex = DUPLEX_HALF;
> +
> + speed = mii_reg & MIIM_AUX_CNTRL_STAT_SPEED_MASK;
> + speed = speed >> MIIM_AUX_CNTRL_STAT_SPEED_POS;
> +
> + switch (speed) {
> + case MIIM_AUX_CNTRL_STAT_SPEED_1000M:
> + phydev->speed = SPEED_1000;
> + break;
> + case MIIM_AUX_CNTRL_STAT_SPEED_100M:
> + phydev->speed = SPEED_100;
> + break;
> + case MIIM_AUX_CNTRL_STAT_SPEED_10M:
> + phydev->speed = SPEED_10;
> + break;
> + default:
> + phydev->speed = SPEED_10;
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static int mscc_startup(struct phy_device *phydev)
> +{
> + int ret;
Please use the ubiquitous 'retval' for this variable name.
> +
> + ret = genphy_update_link(phydev);
> + if (ret)
> + return ret;
> + return mscc_parse_status(phydev);
> +}
> +
> +static int mscc_phy_soft_reset(struct phy_device *phydev)
> +{
> + int rc = 0;
Same here, use retval.
> + u16 timeout = MSCC_PHY_RESET_TIMEOUT;
> + u16 reg_val = 0;
> +
> + phy_write(phydev, MDIO_DEVAD_NONE, MSCC_EXT_PAGE_ACCESS,
> + MSCC_PHY_PAGE_STD);
> +
> + reg_val = phy_read(phydev, MDIO_DEVAD_NONE, MII_BMCR);
> + reg_val |= BMCR_RESET;
> + phy_write(phydev, MDIO_DEVAD_NONE, MII_BMCR, reg_val);
> +
> + reg_val = phy_read(phydev, MDIO_DEVAD_NONE, MII_BMCR);
> +
> + while ((reg_val & BMCR_RESET) && (timeout > 0)) {
> + reg_val = phy_read(phydev, MDIO_DEVAD_NONE, MII_BMCR);
> + timeout--;
> + udelay(1000); /* 1 ms */
> + }
> +
> + if (timeout == 0) {
> + printf("MSCC PHY Soft_Reset Error: mac i/f = 0x%x\n",
> + phydev->interface);
> + rc = -ETIME;
> + }
> +
> + return rc;
> +}
> +
> +static int vsc8531_vsc8541_mac_config(struct phy_device *phydev)
> +{
> + int rc = 0;
This variable is never used. Just return 0 at the end.
> + u16 reg_val = 0;
> + u16 mac_if = 0;
> + u16 rx_clk_out = 0;
> +
> + /* For VSC8530/31 the only MAC modes are RMII/RGMII. */
> + /* For VSC8540/41 the only MAC modes are (G)MII and RMII/RGMII. */
> + /* Setup MAC Configuration */
> + switch (phydev->interface) {
> + case PHY_INTERFACE_MODE_MII:
> + case PHY_INTERFACE_MODE_GMII:
> + /* Set Reg23.12:11=0 */
> + mac_if = MAC_IF_SELECTION_GMII;
> + /* Set Reg20E2.11=1 */
> + rx_clk_out = RX_CLK_OUT_DISABLE;
> + break;
> +
> + case PHY_INTERFACE_MODE_RMII:
> + /* Set Reg23.12:11=1 */
> + mac_if = MAC_IF_SELECTION_RMII;
> + /* Set Reg20E2.11=0 */
> + rx_clk_out = RX_CLK_OUT_NORMAL;
> + break;
> +
> + case PHY_INTERFACE_MODE_RGMII:
> + /* Set Reg23.12:11=2 */
> + mac_if = MAC_IF_SELECTION_RGMII;
> + /* Set Reg20E2.11=0 */
> + rx_clk_out = RX_CLK_OUT_NORMAL;
> + break;
> +
> + default:
> + printf("MSCC PHY - INVALID MAC i/f Config: mac i/f = 0x%x\n",
> + phydev->interface);
> + return -EINVAL;
> + }
> +
> + phy_write(phydev, MDIO_DEVAD_NONE, MSCC_EXT_PAGE_ACCESS,
> + MSCC_PHY_PAGE_STD);
> +
> + /* Read Reg23 */
> + reg_val = phy_read(phydev, MDIO_DEVAD_NONE,
> + MSCC_PHY_EXT_PHY_CNTL_1);
> + /* Set MAC i/f bits Reg23.12:11 */
> + reg_val = bitfield_replace(reg_val, MAC_IF_SELECTION_POS,
> + MAC_IF_SELECTION_WIDTH, mac_if);
> + /* Update Reg23.12:11 */
> + phy_write(phydev, MDIO_DEVAD_NONE,
> + MSCC_PHY_EXT_PHY_CNTL_1, reg_val);
> + /* Setup ExtPg_2 Register Access */
> + phy_write(phydev, MDIO_DEVAD_NONE,
> + MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_EXT2);
> + /* Read Reg20E2 */
> + reg_val = phy_read(phydev, MDIO_DEVAD_NONE,
> + MSCC_PHY_RGMII_CNTL);
> + reg_val = bitfield_replace(reg_val, RX_CLK_OUT_POS,
> + RX_CLK_OUT_WIDTH, rx_clk_out);
> + /* Update Reg20E2.11 */
> + phy_write(phydev, MDIO_DEVAD_NONE,
> + MSCC_PHY_RGMII_CNTL, reg_val);
> + /* Before leaving - Change back to Std Page Register Access */
> + phy_write(phydev, MDIO_DEVAD_NONE, MSCC_EXT_PAGE_ACCESS,
> + MSCC_PHY_PAGE_STD);
> +
> + return rc;
> +}
> +
> +static int vsc8531_config(struct phy_device *phydev)
> +{
> + u16 reg_val;
> + u16 rmii_clk_out;
> + enum vsc_phy_rgmii_skew rx_clk_skew = VSC_PHY_RGMII_DELAY_1700_PS;
> + enum vsc_phy_rgmii_skew tx_clk_skew = VSC_PHY_RGMII_DELAY_800_PS;
> + enum vsc_phy_clk_slew edge_rate = VSC_PHY_CLK_SLEW_RATE_4;
> +
> + /* For VSC8530/31 and VSC8540/41 the init scripts are the same */
> + mscc_vsc8531_vsc8541_init_scripts(phydev);
> +
> + /* For VSC8530/31 the only MAC modes are RMII/RGMII. */
> + switch (phydev->interface) {
> + case PHY_INTERFACE_MODE_RMII:
> + case PHY_INTERFACE_MODE_RGMII:
> + vsc8531_vsc8541_mac_config(phydev);
> + mscc_phy_soft_reset(phydev);
> + break;
> + default:
> + printf("PHY 8530/31 MAC i/f Config Error: mac i/f = 0x%x\n",
> + phydev->interface);
> + return -EINVAL;
> + }
> + /* Default RMII Clk Output to 0=OFF/1=ON */
> + rmii_clk_out = 0;
> +
> + phy_write(phydev, MDIO_DEVAD_NONE, MSCC_EXT_PAGE_ACCESS,
> + MSCC_PHY_PAGE_EXT2);
> + reg_val = phy_read(phydev, MDIO_DEVAD_NONE, MSCC_PHY_RGMII_CNTL);
> +
> + /* Reg20E2 - Update RGMII RX_Clk Skews. */
> + reg_val = bitfield_replace(reg_val, RGMII_RX_CLK_DELAY_POS,
> + RGMII_RX_CLK_DELAY_WIDTH, rx_clk_skew);
> + /* Reg20E2 - Update RGMII TX_Clk Skews. */
> + reg_val = bitfield_replace(reg_val, RGMII_TX_CLK_DELAY_POS,
> + RGMII_TX_CLK_DELAY_WIDTH, tx_clk_skew);
> +
> + phy_write(phydev, MDIO_DEVAD_NONE, MSCC_PHY_RGMII_CNTL, reg_val);
> +
> + reg_val = phy_read(phydev, MDIO_DEVAD_NONE, MSCC_PHY_WOL_MAC_CONTROL);
> + /* Reg27E2 - Update Clk Slew Rate. */
> + reg_val = bitfield_replace(reg_val, EDGE_RATE_CNTL_POS,
> + EDGE_RATE_CNTL_WIDTH, edge_rate);
> + /* Reg27E2 - Update RMII Clk Out. */
> + reg_val = bitfield_replace(reg_val, RMII_CLK_OUT_ENABLE_POS,
> + RMII_CLK_OUT_ENABLE_WIDTH, rmii_clk_out);
> + /* Update Reg27E2 */
> + phy_write(phydev, MDIO_DEVAD_NONE, MSCC_PHY_WOL_MAC_CONTROL, reg_val);
> + phy_write(phydev, MDIO_DEVAD_NONE, MSCC_EXT_PAGE_ACCESS,
> + MSCC_PHY_PAGE_STD);
> +
> + return genphy_config_aneg(phydev);
> +}
> +
> +static int vsc8541_config(struct phy_device *phydev)
> +{
> + u16 reg_val;
> + u16 rmii_clk_out;
> + enum vsc_phy_rgmii_skew rx_clk_skew = VSC_PHY_RGMII_DELAY_1700_PS;
> + enum vsc_phy_rgmii_skew tx_clk_skew = VSC_PHY_RGMII_DELAY_800_PS;
> + enum vsc_phy_clk_slew edge_rate = VSC_PHY_CLK_SLEW_RATE_4;
> +
> + /* For VSC8530/31 and VSC8540/41 the init scripts are the same */
> + mscc_vsc8531_vsc8541_init_scripts(phydev);
> +
> + /* For VSC8540/41 the only MAC modes are (G)MII and RMII/RGMII. */
> + switch (phydev->interface) {
> + case PHY_INTERFACE_MODE_MII:
> + case PHY_INTERFACE_MODE_GMII:
> + case PHY_INTERFACE_MODE_RMII:
> + case PHY_INTERFACE_MODE_RGMII:
> + vsc8531_vsc8541_mac_config(phydev);
> + mscc_phy_soft_reset(phydev);
> + break;
> + default:
> + printf("PHY 8541 MAC i/f config Error: mac i/f = 0x%x\n",
> + phydev->interface);
> + return -EINVAL;
> + }
> + /* Default RMII Clk Output to 0=OFF/1=ON */
> + rmii_clk_out = 0;
> +
> + phy_write(phydev, MDIO_DEVAD_NONE, MSCC_EXT_PAGE_ACCESS,
> + MSCC_PHY_PAGE_EXT2);
> + reg_val = phy_read(phydev, MDIO_DEVAD_NONE, MSCC_PHY_RGMII_CNTL);
> + /* Reg20E2 - Update RGMII RX_Clk Skews. */
> + reg_val = bitfield_replace(reg_val, RGMII_RX_CLK_DELAY_POS,
> + RGMII_RX_CLK_DELAY_WIDTH, rx_clk_skew);
> + /* Reg20E2 - Update RGMII TX_Clk Skews. */
> + reg_val = bitfield_replace(reg_val, RGMII_TX_CLK_DELAY_POS,
> + RGMII_TX_CLK_DELAY_WIDTH, tx_clk_skew);
> + phy_write(phydev, MDIO_DEVAD_NONE, MSCC_PHY_RGMII_CNTL, reg_val);
> +
> + reg_val = phy_read(phydev, MDIO_DEVAD_NONE, MSCC_PHY_WOL_MAC_CONTROL);
> + /* Reg27E2 - Update Clk Slew Rate. */
> + reg_val = bitfield_replace(reg_val, EDGE_RATE_CNTL_POS,
> + EDGE_RATE_CNTL_WIDTH, edge_rate);
> + /* Reg27E2 - Update RMII Clk Out. */
> + reg_val = bitfield_replace(reg_val, RMII_CLK_OUT_ENABLE_POS,
> + RMII_CLK_OUT_ENABLE_WIDTH, rmii_clk_out);
> + /* Update Reg27E2 */
> + phy_write(phydev, MDIO_DEVAD_NONE, MSCC_PHY_WOL_MAC_CONTROL, reg_val);
> + phy_write(phydev, MDIO_DEVAD_NONE, MSCC_EXT_PAGE_ACCESS,
> + MSCC_PHY_PAGE_STD);
> +
> + return genphy_config_aneg(phydev);
> +}
> +
> +static struct phy_driver VSC8530_driver = {
> + .name = "Microsemi VSC8530",
> + .uid = PHY_ID_VSC8530,
> + .mask = 0x000ffff0,
> + .features = PHY_BASIC_FEATURES,
> + .config = &vsc8531_config,
> + .startup = &mscc_startup,
> + .shutdown = &genphy_shutdown,
> +};
> +
> +static struct phy_driver VSC8531_driver = {
> + .name = "Microsemi VSC8531",
> + .uid = PHY_ID_VSC8531,
> + .mask = 0x000ffff0,
> + .features = PHY_GBIT_FEATURES,
> + .config = &vsc8531_config,
> + .startup = &mscc_startup,
> + .shutdown = &genphy_shutdown,
> +};
> +
> +static struct phy_driver VSC8540_driver = {
> + .name = "Microsemi VSC8540",
> + .uid = PHY_ID_VSC8540,
> + .mask = 0x000ffff0,
> + .features = PHY_BASIC_FEATURES,
> + .config = &vsc8541_config,
> + .startup = &mscc_startup,
> + .shutdown = &genphy_shutdown,
> +};
> +
> +static struct phy_driver VSC8541_driver = {
> + .name = "Microsemi VSC8541",
> + .uid = PHY_ID_VSC8541,
> + .mask = 0x000ffff0,
> + .features = PHY_GBIT_FEATURES,
> + .config = &vsc8541_config,
> + .startup = &mscc_startup,
> + .shutdown = &genphy_shutdown,
> +};
> +
> +int phy_mscc_init(void)
> +{
> + phy_register(&VSC8530_driver);
> + phy_register(&VSC8531_driver);
> + phy_register(&VSC8540_driver);
> + phy_register(&VSC8541_driver);
> +
> + return 0;
> +}
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index 80bdfb6..8db6574 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -512,6 +512,9 @@ int phy_init(void)
> #ifdef CONFIG_PHY_XILINX
> phy_xilinx_init();
> #endif
> +#ifdef CONFIG_PHY_MSCC
> + phy_mscc_init();
> +#endif
>
> return 0;
> }
> diff --git a/include/config_phylib_all_drivers.h b/include/config_phylib_all_drivers.h
> index 12828c6..d2965ed 100644
> --- a/include/config_phylib_all_drivers.h
> +++ b/include/config_phylib_all_drivers.h
> @@ -14,6 +14,7 @@
>
> #ifdef CONFIG_PHYLIB
>
> +#define CONFIG_PHY_MSCC
> #define CONFIG_PHY_VITESSE
> #define CONFIG_PHY_MARVELL
> #define CONFIG_PHY_MICREL
> diff --git a/include/configs/am335x_evm.h b/include/configs/am335x_evm.h
> index ec70b72..694e249 100644
> --- a/include/configs/am335x_evm.h
> +++ b/include/configs/am335x_evm.h
> @@ -433,6 +433,9 @@
> #define CONFIG_PHY_SMSC
> /* Enable Atheros phy driver */
> #define CONFIG_PHY_ATHEROS
> +/* Enable MSCC phy driver */
> +#define CONFIG_PHY_MSCC
> +
>
> /*
> * NOR Size = 16 MiB
> diff --git a/include/phy.h b/include/phy.h
> index 268d9a1..5477496 100644
> --- a/include/phy.h
> +++ b/include/phy.h
> @@ -266,6 +266,7 @@ int phy_teranetics_init(void);
> int phy_ti_init(void);
> int phy_vitesse_init(void);
> int phy_xilinx_init(void);
> +int phy_mscc_init(void);
>
> int board_phy_config(struct phy_device *phydev);
> int get_phy_id(struct mii_dev *bus, int addr, int devad, u32 *phy_id);
> --
> 1.9.1
>
>
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
^ permalink raw reply [flat|nested] 6+ messages in thread