From mboxrd@z Thu Jan  1 00:00:00 1970
From: Alexey Brodkin <Alexey.Brodkin@synopsys.com>
Date: Thu, 8 Dec 2016 12:24:22 +0000
Subject: [U-Boot] [PATCH 1/1] Adding MSCC
	PHY-VSC8530-VSC8531-VSC8540-VSC8541
In-Reply-To: <90919B351379A448A858E3A967093A5C42691492@avsrvexchmbx1.microsemi.net>
References: <90919B351379A448A858E3A967093A5C42691492@avsrvexchmbx1.microsemi.net>
Message-ID: <1481199791.2960.73.camel@synopsys.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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